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

binary-reader-ir: Track usage of exception handling in features_used #2496

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

SoniEx2
Copy link
Contributor

@SoniEx2 SoniEx2 commented Oct 29, 2024

No description provided.

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Oct 29, 2024

@sbc100

Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

How about "binary-reader-ir: track usage of exception handling in features_used"?

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Oct 29, 2024

sure!

@keithw
Copy link
Member

keithw commented Oct 29, 2024

lgtm, but shouldn't this be part of #2470 ?

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Oct 29, 2024

we're splitting that into like 100 tiny commits because it makes review easier apparently

@SoniEx2 SoniEx2 changed the title binary-reader-ir: mark exceptions used as needed binary-reader-ir: track usage of exception handling in features_used Oct 29, 2024
@sbc100
Copy link
Member

sbc100 commented Oct 29, 2024

we're splitting that into like 100 tiny commits because it makes review easier apparently

You maybe don't have to go this fine grained. I'm general I think a good rule of thumb is to break things into changes that can be independently tested. In this case we don't have any test changes so maybe this is on the small side? lgtm either way.

@sbc100 sbc100 changed the title binary-reader-ir: track usage of exception handling in features_used binary-reader-ir: Track usage of exception handling in features_used Oct 29, 2024
@sbc100 sbc100 merged commit 22b8252 into WebAssembly:main Oct 29, 2024
18 checks passed
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