-
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
Move all js library decisions to a single location #8428
Conversation
@@ -3156,8 +3156,6 @@ def process_libraries(libs, lib_dirs, settings_changes, input_files): | |||
if not found: | |||
libraries += shared.Building.path_to_system_js_libraries(lib) | |||
|
|||
# Certain linker flags imply some link libraries to be pulled in by default. | |||
libraries += shared.Building.path_to_system_js_libraries_for_settings(settings_changes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can also remove the settings_changes arg from this function too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. you can also just remove Found completely and return on line 3150?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah, but then we'd need to duplicate the 'SYSTEM_JS_LIBRARIES="' + ','.join(libraries) + '"'
line. Not sure which I prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok lgtm
…n-core#8428)" This reverts commit fb9b1dd.
…n-core#8428)" This reverts commit fb9b1dd.
This was spread out in 2 places before, needlessly.