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

emscripten: Add JS library dependencies using EM_JS_DEPS macro #7935

Merged
merged 1 commit into from
Jul 6, 2023

Conversation

sbc100
Copy link
Contributor

@sbc100 sbc100 commented Jul 5, 2023

@icculus
Copy link
Collaborator

icculus commented Jul 6, 2023

Oh wow, I've been bit by this problem myself; EM_JS_DEPS is going to be a life-saver for sure. :)

I saw this was force-pushed a few times; let me know when you're done and I'll merge this!

@sbc100
Copy link
Contributor Author

sbc100 commented Jul 6, 2023

I'm done if the CI passes!

@icculus
Copy link
Collaborator

icculus commented Jul 6, 2023

GitHub seems to be having a build-farm issue at the moment, but we'll get this in when they fix it. :)

@icculus icculus merged commit 0422434 into libsdl-org:SDL2 Jul 6, 2023
37 checks passed
@sezero
Copy link
Contributor

sezero commented Jul 6, 2023

Are the changes not needed in SDL3 ?

@icculus
Copy link
Collaborator

icculus commented Jul 6, 2023

Oh, whoops, I thought this was SDL3. I'll cherry-pick it over.

@sbc100
Copy link
Contributor Author

sbc100 commented Jul 6, 2023

Oh, whoops, I thought this was SDL3. I'll cherry-pick it over.

Sorry, whats the policy on committing to with SDL2 vs main branch?

@sbc100 sbc100 deleted the add_em_js_deps branch July 6, 2023 22:54
@slouken
Copy link
Collaborator

slouken commented Jul 6, 2023

In general development goes into SDL3, and is cherry picked into SDL2, but we're flexible.

@icculus
Copy link
Collaborator

icculus commented Jul 6, 2023

This is in for SDL3 in b42cb1c; should we kick this into 2.28.2, as well?

@slouken
Copy link
Collaborator

slouken commented Jul 6, 2023

No, let's leave it as-is for 2.28.1 unless it's critical.

@ericoporto
Copy link
Contributor

I think this is safe to merge in release-2.28 branch so it gets in a 2.28.5 release.

@slouken
Copy link
Collaborator

slouken commented Oct 9, 2023

This is already in the 2.28.4 release.

@ericoporto
Copy link
Contributor

ericoporto commented Oct 9, 2023

This is already in the 2.28.4 release

Ah, ok, found the commit, it was in September:

7d80e20

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants