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

Implement the "not" condition support #1711

Merged
merged 3 commits into from
Apr 16, 2024

Conversation

yoshisatoyanagisawa
Copy link
Contributor

@yoshisatoyanagisawa yoshisatoyanagisawa commented Mar 19, 2024

This is a specification update on supporting the "not" condition, which is also written in
https://github.com/WICG/service-worker-static-routing-api/blob/main/final-form.md.

The "not" condition is used for reversing the condition inside for ease of writing a matching rule for a reversed condition.


Preview | Diff

aarongable pushed a commit to chromium/chromium that referenced this pull request Mar 21, 2024
This CL changes WebIDL to support the not condition, and makes it
tested.
Since the not condition has not been written in the specification yet,
it is implemented behind the flag.

Explainer: https://github.com/WICG/service-worker-static-routing-api/blob/main/final-form.md
Design Doc: https://docs.google.com/document/d/1B2D8w_2M4j9CZC1VccBOAhfCdiRP3yPCSV5UKKDhU70/
Specification: w3c/ServiceWorker#1711

Bug: 328565554
Change-Id: I0b50a26530e6aa273ef4b8c20181104ad5e3591c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5367792
Commit-Queue: Yoshisato Yanagisawa <[email protected]>
Reviewed-by: Minoru Chikamune <[email protected]>
Reviewed-by: Takashi Toyoshima <[email protected]>
Reviewed-by: Kouhei Ueno <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1276118}
@yoshisatoyanagisawa
Copy link
Contributor Author

@domenic @sisidovski Will you take a look at the spec change?

Copy link
Contributor

@domenic domenic left a comment

Choose a reason for hiding this comment

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

LGTM with one fix

docs/index.bs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@yoshisatoyanagisawa yoshisatoyanagisawa left a comment

Choose a reason for hiding this comment

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

Thanks for the review.

docs/index.bs Outdated Show resolved Hide resolved
@yoshisatoyanagisawa
Copy link
Contributor Author

@mkruisselbrink Will you review this?

Copy link
Contributor

@sisidovski sisidovski left a comment

Choose a reason for hiding this comment

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

LGTM

@yoshisatoyanagisawa
Copy link
Contributor Author

@mkruisselbrink Do you have any concerns on this PR? Please feel free to point them out.

Copy link
Collaborator

@mkruisselbrink mkruisselbrink left a comment

Choose a reason for hiding this comment

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

lgtm, sorry for the delay in reviewing

@mkruisselbrink mkruisselbrink merged commit 8d64a5c into w3c:main Apr 16, 2024
2 checks passed
github-actions bot added a commit that referenced this pull request Apr 16, 2024
SHA: 8d64a5c
Reason: push, by mkruisselbrink

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@yoshisatoyanagisawa yoshisatoyanagisawa deleted the not_condition branch April 16, 2024 23:23
github-actions bot added a commit to asleekgeek/ServiceWorker that referenced this pull request Apr 17, 2024
SHA: 8d64a5c
Reason: push, by pull[bot]

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

4 participants