-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Rename JS library files from library_foo.js to libfoo.js #23348
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Marking as draft for now. |
sbc100
force-pushed
the
library_rename
branch
3 times, most recently
from
January 17, 2025 00:26
64743d3
to
49a1a75
Compare
sbc100
changed the title
Rename system library files from library_foo.js to libfoo.js
Rename JS library files from library_foo.js to libfoo.js
Jan 17, 2025
kripken
reviewed
Jan 17, 2025
kripken
reviewed
Jan 17, 2025
sbc100
force-pushed
the
library_rename
branch
from
January 17, 2025 19:20
49a1a75
to
31a7e36
Compare
sbc100
force-pushed
the
library_rename
branch
4 times, most recently
from
January 17, 2025 19:48
5a3327c
to
cf00d64
Compare
sbc100
added a commit
that referenced
this pull request
Jan 17, 2025
Files references via #include can now either relative to the including file or come from the system library. Previously only relative paths were allowed. Also we now give a nicer error message when the file is not found. Split out from #23348
sbc100
force-pushed
the
library_rename
branch
from
January 17, 2025 22:02
cf00d64
to
a24778e
Compare
Much cleaner PR now that #23449 landed |
kripken
approved these changes
Jan 17, 2025
At the same time, move all the library file into a separate `src/lib` directory. This helps distinguish library files from other files in the `lib` directory, which is currently very overcrowded/overloaded. These files were also includ-able via `-lfoo` and we previously had to use a special case for system libraries where `library_foo.js` could be used in addition to more standed `libfoo.js`. This change removes to special case. This only file that was not renamed using the simple pattern was `library.py` itself which becomes `libcore.py`. I also considered `libemscripten.js`. To some this may seem like needless churn, but for years now the `src` directory (IMHO) has been asking for cleanup. I think having files logically separated into directories helps a lot with understanding the project structure for both newcomers and old timers alike. As a followup, I am also considering moving all the runtime files into `src/include` (since these are all files that get `#included` into the generated code directly. That would leave the `src` directory containing only the JS compiler code itself which I think is another important distinction.
sbc100
force-pushed
the
library_rename
branch
from
January 17, 2025 23:06
a24778e
to
e839ead
Compare
Added a ChangeLog entry |
lgtm |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At the same time, move all the library file into a separate
src/lib
directory. This helps distinguish library files from other files
in the
lib
directory, which is currently very overcrowded/overloaded.These files were also includ-able via
-lfoo
and we previously had touse a special case for system libraries where
library_foo.js
could beused in addition to more standed
libfoo.js
. This change removes tospecial case.
This only file that was not renamed using the simple pattern was
library.py
itself which becomeslibcore.py
. I also consideredlibemscripten.js
.To some this may seem like needless churn, but for years now the
src
directory (IMHO) has been asking for cleanup. I think having files
logically separated into directories helps a lot with understanding the
project structure for both newcomers and old timers alike.
As a followup, I am also considering moving all the runtime files into
src/include
(since these are all files that get#included
into thegenerated code directly. That would leave the
src
directorycontaining only the JS compiler code itself which I think is another
important distinction.