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(ext/url): two-argument delete() and has() #19011

Closed

Conversation

lino-levan
Copy link
Contributor

Closes #19000. Not sure if it was a good idea to update the WPT in this PR but /shrug. I could always remove it.

Copy link
Collaborator

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

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

Updated WPT brought with it test failures.

@lino-levan
Copy link
Contributor Author

@aapoalas should I revert WPT test changes? The actual changes pass the two new tests added to url. Not sure what the best move is here.

@aapoalas
Copy link
Collaborator

aapoalas commented May 6, 2023

I'm not quite sure either. Do you know / understand from the output and changes if the now-failing tests are new ones, or old ones that have now started to fail?

If these are new tests, then I would just add expectations of failure for those. If they are old tests that have started failing, then... Well, then I guess the correct choice would be to create issues for each failing test (file).

Any new feature, like this, should have tests so in that sense updating the WPT no matter the tests makes sense to me.

@crowlKats
Copy link
Member

Please update WPT in a separate PR

@lino-levan
Copy link
Contributor Author

Will do.

@lino-levan
Copy link
Contributor Author

Waiting on #19061 to be resolved since it's looking like there may be a lot of conflicts otherwise.

@lino-levan lino-levan closed this Jun 29, 2023
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.

Add value argument to URLSearchParams's has() and delete()
3 participants