Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

split twinkle.js into two files #2077

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

NovemLinguae
Copy link
Member

@NovemLinguae NovemLinguae commented Nov 25, 2024

One file (twinkleutil.js, new file) for functions, one file (twinkle.js, original file) for everything else.

For unit test reasons. The non-function code will get in the way if we want to write unit tests for the functions.

Refactoring. No-op.


Todo

One file for functions, one file for everything else.

For unit test reasons. The non-function code will get in the way if we want to write unit tests for the functions.

Refactoring. No-op.
@github-actions github-actions bot added Module: twinkle The twinkle.js global gadget file Gadget labels Nov 25, 2024
@github-actions github-actions bot added the tests label Nov 25, 2024
@NovemLinguae NovemLinguae marked this pull request as draft November 25, 2024 02:09
@siddharthvp
Copy link
Member

The load order matters - the file defining Twinkle global needs to be first. It's okay for this file to call functions declared in the other, as long as these calls are in async callbacks rather than at the top level (0 indent level).

@siddharthvp
Copy link
Member

As the intent is to increase coverage, I'd suggest putting all top-level code into functions and continuing to call the file twinkle.js. The function which starts the loading can be invoked from a new file, say twinkleload.js, which would be only one line and doesn't contain anything else. Only twinkleload.js would need to be skipped from coverage reporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gadget Module: twinkle The twinkle.js global gadget file tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants