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

Match simple-shared-worker/multiply.js to use same event-handler loop as simple-web-worker/main.js #284

Merged
merged 6 commits into from
Dec 30, 2024

Conversation

Marcial1234
Copy link
Contributor

@Marcial1234 Marcial1234 commented Nov 20, 2024

Caught this small inconsistency - on one SW each event handler is attached separately, in the other they're done inside a loop.
Less code seems better.

See local reference

Perhaps the external reference could be reduced/simplified as well?

@Marcial1234 Marcial1234 requested a review from a team as a code owner November 20, 2024 19:24
@Marcial1234 Marcial1234 requested review from bsmth and removed request for a team November 20, 2024 19:24
@bsmth
Copy link
Member

bsmth commented Dec 12, 2024

Thanks a lot for opening this one. Related PR landed similar fixes in:

@bsmth
Copy link
Member

bsmth commented Dec 12, 2024

@Marcial1234 Would you like to open a pull request in mdn/content that makes this consistent there, too?

@Marcial1234
Copy link
Contributor Author

Marcial1234 commented Dec 12, 2024 via email

@Marcial1234
Copy link
Contributor Author

mdn/content#37208 @bsmth

Copy link
Member

@bsmth bsmth left a comment

Choose a reason for hiding this comment

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

Looking great, I'm going to merge this one and the content improvements shortly! Well done and thank you!

@bsmth bsmth merged commit 5970eb3 into mdn:main Dec 30, 2024
3 checks passed
@bsmth bsmth mentioned this pull request Dec 31, 2024
1 task
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