Refactor: executable script consistency #5890
Conversation
ghost
added
bulk_update 📦
queengooborg
changed the title
|
I think we can simplify further and remove the shebangs everywhere. Shebangs are needed when a file is executed directly, like When doing something like |
There was a problem hiding this comment.
Hmm, so going deeper into review I see there are a bunch of things being changed here. How about breaking out the simple "make file headers consistent" bits separately, and doing the other things as separate PRs that explain in more what's being changed and why?
scripts/fix-browser-order.js
Outdated
| @@ -53,52 +53,4 @@ const fixBrowserOrder = filename => { | |||
| } | |||
| }; | |||
|
|
|||
| if (require.main === module) { | |||
There was a problem hiding this comment.
What's going on with this removal?
There was a problem hiding this comment.
This file isn't really intended to be run on its own, so that's why I've removed this clause.
|
Sounds good -- I've marked this PR as a draft to use as a breakdown PR. I'll make individual changes in other PRs! |
|
This pull request has merge conflicts that must be resolved before we can merge this. |
|
This pull request has merge conflicts that must be resolved before we can merge this. |
queengooborg
changed the title
|
This PR now has a small enough scope that it can be reviewed and merged on its own, so I've updated the description and title to match the scope of this PR! |
| tests.forEach(({ features, matches, misses }) => { | ||
| features.forEach((feature) => testToken(lookup(feature), matches, misses)); | ||
| }); | ||
| module.exports = testRegexes; |
There was a problem hiding this comment.
Why does this script not have an if (require.main === module) { block?
There was a problem hiding this comment.
This script isn't supposed to be run on its own, but rather as a Mocha unittest. It was never actually hooked up though, and I'm not too worried about hooking it up now since it's going to be removed in #15781!
This PR is a subset of #5654 and #5655 (and partially from #5626 (comment)). Throughout our scripts, we have a mix of scripts designed to be executable on their own or to be imported into other scripts. Unfortunately, we haven't been consistent about using
require.main === moduleto state execution intent. This PR performs the following:require.main === moduleif-clause to make all executable scripts importable