Skip to content

Move some Module object assertions. NFC #23995

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

Merged
merged 1 commit into from
Mar 26, 2025

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Mar 26, 2025

This moves these assertion alongside other similar checks.

@sbc100 sbc100 requested review from kripken and brendandahl March 26, 2025 22:37
@sbc100 sbc100 force-pushed the module_env_check branch from ff3fcf0 to 23ee272 Compare March 26, 2025 22:42
Copy link
Collaborator

@brendandahl brendandahl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+ but update commit message since this moves two things.

@sbc100 sbc100 changed the title Move Module['ENVIRONMENT'] check. NFC Move some Module object assertions. NFC Mar 26, 2025
This moves this assertion alongside other similar checks.
@sbc100 sbc100 force-pushed the module_env_check branch from 23ee272 to c2aefa3 Compare March 26, 2025 23:25
@sbc100 sbc100 enabled auto-merge (squash) March 26, 2025 23:26
@sbc100 sbc100 disabled auto-merge March 26, 2025 23:42
@sbc100 sbc100 merged commit 7a2ee39 into emscripten-core:main Mar 26, 2025
9 of 11 checks passed
@sbc100 sbc100 deleted the module_env_check branch March 26, 2025 23:42
@kripken
Copy link
Member

kripken commented Mar 27, 2025

It looks like other.test_emcc_4 failed on CI here, and is now failing on the rollers,

https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8719235225433347489/+/u/Emscripten_testsuite__other_/stdout

Maybe it looked like a flake but it wasn't?

sbc100 added a commit to sbc100/emscripten that referenced this pull request Mar 27, 2025
sbc100 added a commit that referenced this pull request Mar 27, 2025
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.

3 participants