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

feat: add new web lock api interface #3604

Merged
merged 5 commits into from Sep 16, 2023

Conversation

Solo-steven
Copy link
Contributor

@Solo-steven Solo-steven commented Sep 10, 2023

fixes #3456.
this is the first time I have contributed to this repo, If there is some definition that is wrong, please tell me! thanks ~

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Not sure exactly what happened here, did you check the CI failure?
Could you take a look at the documentation and see if that generates the code correctly?

  • Looking at the reference there are a couple of APIs still missing.
  • A changelog entry is missing.

It would also be nice if you link your sources. E.g. double-checking the Mozilla implementation to see if any functions are throwing.

Otherwise LGTM!

@Solo-steven
Copy link
Contributor Author

Hi @daxpedda, thanks for your review, It is my fault that I wrote the gen_* file instead of using the auto codegen command, Now I have switched to auto codegen, and All the CI tests are passed.
Since MDN only have LockManager and Lock document, I only add the MDN link to gen_lock and gen_lockManager, if there is any wrong, please tell me ~ thanks ~

@daxpedda
Copy link
Collaborator

Since MDN only have LockManager and Lock document, I only add the MDN link to gen_lock and gen_lockManager, if there is any wrong, please tell me ~ thanks ~

I only meant posting it here in the issue! But that's for the next time, I already got the links and posted them above 😉.

Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

LGTM!
Changelog entry is missing.

@daxpedda daxpedda added the waiting for author Waiting for author to respond label Sep 13, 2023
@Solo-steven
Copy link
Contributor Author

Hi @daxpedda thanks for the reminder, I added the entry to the changelog, if the entry description is not correct, please tell me, thanks ~

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Thanks!

@daxpedda daxpedda merged commit e617fca into rustwasm:main Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for author Waiting for author to respond
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Web Locks API?
2 participants