Add a section on other specs integrating with URLPattern #199

jeremyroman · 2023-11-21T05:08:55Z

A section which covers how other specifications should use URLPattern and how developer-facing APIs should work is added, along with helpful algorithms.

One of these is whether a pattern has regexp groups (which may require an ECMAScript regexp engine); a corresponding WebIDL attribute is added to expose this property to authors as well.

At least two implementers are interested (and none opposed):
- Google Chrome
- WebKit
Tests are written and can be reviewed and commented upon at:
- https://chromium-review.googlesource.com/c/chromium/src/+/5054393
Implementation bugs are filed:
- Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1504650, https://bugs.chromium.org/p/chromium/issues/detail?id=1504683
- Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1731418 (vendor does not yet implement the spec)
- WebKit: [no known URLPattern bug] (vendor does not yet implement the spec)
- Deno: URLPattern: Support hasRegExpGroups. denoland/rust-urlpattern#42
- kenchris/urlpattern-polyfill: Support hasRegExpGroups kenchris/urlpattern-polyfill#125
MDN issue is filed: URLPattern: hasRegExpGroups mdn/content#30640
The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

Preview | Diff

A section which covers how other specifications should use URLPattern and how developer-facing APIs should work is added, along with helpful algorithms. One of these is whether a pattern has regexp groups (which may require an ECMAScript regexp engine); a corresponding WebIDL attribute is added to expose this property to authors as well.

jeremyroman · 2023-11-21T05:09:36Z

This relates to #182 and #191.

…rectly

spec.bs

domenic · 2023-11-21T05:50:51Z

spec.bs

+  1. Otherwise, if |rawPattern| is a [=map=], then:
+    1. Let |init| be «[ "`baseURL`" → |serializedBaseURL| ]», representing a dictionary of type {{URLPatternInit}}.
+    1. [=map/For each=] |key| → |value| of |rawPattern|:
+      1. If |key| is not the [=identifier=] of a [=dictionary member=] of {{URLPatternInit}} or one of its [=inherited dictionaries=], |value| is not a [=string=], or the member's type is not declared to be {{USVString}}, then return null.


It's not 100% clear to me that every spec wants the kind of restrictive error handling we've used here. But I feel like we discussed this in the past when writing the speculation rules spec. I'm OK carrying it over for now, until someone raises an issue.

spec.bs

sisidovski · 2023-11-21T07:55:20Z

spec.bs

-  1. Return a new [=component=] whose [=component/pattern string=] is |pattern string|, [=component/regular expression=] is |regular expression|, and [=component/group name list=] is |name list|.
+  1. Let |has regexp groups| be false.
+  1. [=list/For each=] |part| of |part list|:
+    1. If |part|'s [=part/type=] is "<a for=token/type>`regexp`</a>", then set |has regexp groups| to true.


<a for=token/type>`regexp`</a> : part/type is correct instead of token/type?

You're correct; done.

domenic

Found a few more nits, but it's looking quite nice to me.

It might be worth adding (either here or in a followup PR) a section on using URL patterns in HTTP fields, since that's another use case that's coming up in practice. Initial thoughts:

Encourage just using strings for now, and not necessarily accepting dictionaries
The algorithm can be a wrapper around "build a URLPattern from an Infra value" that is only ever called with a string
Give them some help on figuring out exactly which base URL to use. Maybe consider both the request and response header cases?
Maybe omit the realm argument and just use "an implementation-defined realm", because it's quite silly to make HTTP specs deal with realms? We can still have an XXX box, but I'd hate to have the realm as part of the public interface for this particular call site. (Related discussion regarding "implementation-defined realm": "Parse JSON to Infra" algorithms shouldn't require a current JS realm infra#625)

spec.bs

jeremyroman · 2023-11-21T19:20:28Z

Found a few more nits, but it's looking quite nice to me.

It might be worth adding (either here or in a followup PR) a section on using URL patterns in HTTP fields, since that's another use case that's coming up in practice. Initial thoughts:

Encourage just using strings for now, and not necessarily accepting dictionaries

The algorithm can be a wrapper around "build a URLPattern from an Infra value" that is only ever called with a string

Give them some help on figuring out exactly which base URL to use. Maybe consider both the request and response header cases?

Maybe omit the realm argument and just use "an implementation-defined realm", because it's quite silly to make HTTP specs deal with realms? We can still have an XXX box, but I'd hate to have the realm as part of the public interface for this particular call site. (Related discussion regarding "implementation-defined realm": "Parse JSON to Infra" algorithms shouldn't require a current JS realm infra#625)

Yeah, I thought about including HTTP fields in this PR, but wasn't sure if we'd hammered out enough of the details yet.

At surface I would have thought the constructor string syntax would have been the most natural fit, but from what I understand of WICG/compression-dictionary-transport#52 they're currently looking at separate match-path and match-search (or match-query) params (contrary to your proposal here), and it's not clear to me if their reasons for doing so generalize. Like, here, it's somewhat pointless to match on things like protocol and port, but that's not necessarily the case for most hypothetical future header fields.

Besides that one I don't have a clear enough concept of the likely uses to be confident writing guidance, and would probably rather they file an issue or otherwise bring it up to us for now.

domenic · 2023-11-22T01:31:08Z

At surface I would have thought the constructor string syntax would have been the most natural fit, but from what I understand of WICG/compression-dictionary-transport#52 they're currently looking at separate match-path and match-search (or match-query) params (contrary to your proposal here), and it's not clear to me if their reasons for doing so generalize. Like, here, it's somewhat pointless to match on things like protocol and port, but that's not necessarily the case for most hypothetical future header fields.

Hmm, we should discuss with them what's going on. This seems contrary to the design of URL patterns...

domenic

LGTM. It'd be great to get @yoshisatoyanagisawa's review as well, as someone who would be working on the service worker static routing spec, which follows these guidelines.

spec.bs

jeremyroman · 2023-11-22T22:03:31Z

While I'm here, want to note -- the ignoreCase option has the potential to be an obstacle to the cases which can't handle regexp groups, but I think it's actually fine because I think the match algorithm ends up making everything ASCII. Otherwise, the fact that vi regexps do Unicode case folding (so k could match not only uppercase K but also kelvin symbol K , for instance) would be a potential issue for implementations that implemented matching without a regexp engine.

sisidovski

LGTM. I think the new sections explain well. SW static routing API would be able to follow this change.

jeremyroman · 2023-11-24T20:16:25Z

For multi-implementer interest, @annevk is WebKit cool with this? You seemed supportive of at least #191 (which is the real behavior change here right now, since nothing depends on the #182 aspects as yet).

yoshisatoyanagisawa · 2023-11-27T01:23:20Z

spec.bs

+To promote consistency on the web platform, other documents integrating with this specification should adhere to the following guidelines, unless there is good reason to diverge.
+
+1. **Accept shorthands**. Most author patterns will be simple and straightforward. Accordingly, APIs should accept shorthands for those common cases and avoid the need for authors to take additional steps to transform these into complete {{URLPattern}} objects.
+1. **Respect the base URL**. Just as URLs are generally parsed relative to a base URL for their environment (most commonly, a [=document base URL=]), URL patterns should respect this as well. The {{URLPattern}} constructor itself is an exception because it directly exposes the concept itself, similar to how the <a interface spec=URL>URL</a> constructor does not respect the base URL even though the rest of the platform does.


"most commonly" might mean that each specification can choose the base URL to something most relevant to it, right?

Below it suggests the environment settings object's API base URL, which is the document base URL most of the time (in a document), but in a worker it's the global scope's base URL. In an HTTP header it would probably be the request or response URL. Most of the time there's a fairly strong precedent about what URLs are resolved relative to already, and specs should probably follow that unless they have a compelling reason to deviate.

ok. thanks for the clarification.

yoshisatoyanagisawa · 2023-11-27T01:32:45Z

spec.bs

+To accomplish this, specifications should accept {{URLPatternCompatible}} as an argument to an [=operation=] or [=dictionary member=], and process it using the following algorithm, using the appropriate [=environment settings object=]'s [=environment settings object/API base URL=] or equivalent.
+
+<div algorithm>
+  To <dfn for=URLPattern>build a {{URLPattern}} from a WebIDL value</dfn> {{URLPatternCompatible}} |input| given [=/URL=] |baseURL| and [=ECMAScript/realm=]</a> |realm|, perform the following steps:


nit but where is a counterpart of this ?

I don't understand what you're asking; can you elaborate?

Oops, I should have updated this when I noticed the comment can only be given per line X(

In this line, there is </a>. However, I could not find opening of this </a>.

It was an mistakenly left behind when rewriting back and forth between Bikeshed shorthand and <a> syntax. Thanks for catching; removed.

yoshisatoyanagisawa · 2023-11-27T01:38:42Z

spec.bs

+  <p class="XXX">Ideally we wouldn't need a realm here. If we extricate the URL pattern concept from the {{URLPattern}} interface, we won't anymore.</p>
+</div>
+
+This allows authors to concisely specify most patterns, and use the <a constructor for="URLPattern">constructor</a> to access uncommon options if necessary. The implicit use of the base URL is similar to, and consistent with, <cite>HTML</cite>'s [=parse a URL=] algorithm. [[HTML]]


nit but this phrase reminds me #200. It seems that the URLPattern initialization for the short-hand is slightly different from the URL algorithm.

It is indeed slightly different, yeah. Still, I think it's good to keep it broadly similar where we can.

Ok. I hope we can discuss about the topic in #200.

yoshisatoyanagisawa · 2023-11-27T05:14:08Z

Thanks for looping me in, @domenic.
LGTM. I left some minor comments.

domenic · 2023-11-27T05:52:32Z

Given the comments like

Thanks. I'll have to wait for the PR to get resolved before it is something I could reference but as it looks right now, URLPatternCompatible is specific to the Javascript API.

over in WICG/compression-dictionary-transport#52 it seems like it'd be good to add a section on HTTP header usage, either in this PR or as a near-future followup.

This behavior change was originally started in whatwg/urlpattern#182, and follows the spec change in whatwg/urlpattern#199. This CL changes the behavior of the router rule registration in the ServiceWorker Static Routing API, especially when the |urlPattern| condition receives URLPatternInit or USVString. Before this CL, the URLPatternInit input was accepted as it is, that means any unspecified fields are resulted in the wildcards (*). This behavior is inconsistent with the case when |urlPattern| accepts a string. When a string is passed, missing fields are complemented by baseURL, the SW script URL is internally treated as baseURL. After this CL, the URLPatternInit input also internally uses the SW script URL as a baseURL if it's not explicitly provided. This is achieved by the helper method `URLPattern::From()`, which was added in [1]. This change doesn't affect the case when the input is URLPattern, which means the input is the object constructed via `new URLPattern()`. [1] crrev.com/c/5053645 Bug: 1371756 Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b

jeremyroman · 2023-11-27T21:20:15Z

Given the comments like

Thanks. I'll have to wait for the PR to get resolved before it is something I could reference but as it looks right now, URLPatternCompatible is specific to the Javascript API.

over in WICG/compression-dictionary-transport#52 it seems like it'd be good to add a section on HTTP header usage, either in this PR or as a near-future followup.

I agree it would be good to cover once we're agreed on what to do and feel fairly confident it will generalize. I was hoping to avoid entangling these two, so doing HTTP header fields in a followup. It feels slightly silly to write our opinions here and then turn around and use ourselves as authority to back up our own position, but if that's helpful we can do that too.

This behavior change was originally started in whatwg/urlpattern#182, and follows the spec change in whatwg/urlpattern#199. This CL changes the behavior of the router rule registration in the ServiceWorker Static Routing API, especially when the |urlPattern| condition receives URLPatternInit or USVString. Before this CL, the URLPatternInit input was accepted as it is, that means any unspecified fields are resulted in the wildcards (*). This behavior is inconsistent with the case when |urlPattern| accepts a string. When a string is passed, missing fields are complemented by baseURL, the SW script URL is internally treated as baseURL. After this CL, the URLPatternInit input also internally uses the SW script URL as a baseURL if it's not explicitly provided. This is achieved by the helper method `URLPattern::From()`, which was added in [1]. This change doesn't affect the case when the input is URLPattern, which means the input is the object constructed via `new URLPattern()`. [1] crrev.com/c/5053645 Bug: 1371756 Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b

This behavior change was originally started in whatwg/urlpattern#182, and follows the spec change in whatwg/urlpattern#199. This CL changes the behavior of the router rule registration in the ServiceWorker Static Routing API, especially when the |urlPattern| condition receives URLPatternInit or USVString. Before this CL, the URLPatternInit input was accepted as it is, that means any unspecified fields are resulted in the wildcards (*). This behavior is inconsistent with the case when |urlPattern| accepts a string. When a string is passed, missing fields are complemented by baseURL, the SW script URL is internally treated as baseURL. After this CL, the URLPatternInit input also internally uses the SW script URL as a baseURL if it's not explicitly provided. This is achieved by the helper method `URLPattern::From()`, which was added in [1]. This change doesn't affect the case when the input is URLPattern, which means the input is the object constructed via `new URLPattern()`. [1] crrev.com/c/5053645 Bug: 1371756 Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039680 Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Commit-Queue: Shunya Shishido <sisidovski@chromium.org> Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org> Auto-Submit: Shunya Shishido <sisidovski@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229724}

annevk · 2023-11-28T09:26:09Z

@jeremyroman I don't understand the references to Deno in #199 (comment).

WebKit is supportive of figuring out an interface to URLPattern so it can be reused in environments where regular expression engines are a no-no.

I'm not entirely sure that the JavaScript API needs to make the difference observable (as that won't be accessible in those environments), but I guess it's reasonable.

jeremyroman · 2023-11-28T15:57:27Z

@jeremyroman I don't understand the references to Deno in #199 (comment).

Those were supposed to be links to issues in this repository; for some reason Github autolinked to the wrong repository. (???)

WebKit is supportive of figuring out an interface to URLPattern so it can be reused in environments where regular expression engines are a no-no.

I'm not entirely sure that the JavaScript API needs to make the difference observable (as that won't be accessible in those environments), but I guess it's reasonable.

One reason might be registering service worker static routes, where the pattern is registered from a JavaScript environment but evaluated in the fetch handling path.

annevk · 2023-11-28T16:04:21Z

Makes sense, yeah. I do think we need a string-only version (realm-free) of the part you're not explicitly seeking feedback on yet, but we can get there through iteration.

spec.bs

domenic

You should also export the "match" definition https://urlpattern.spec.whatwg.org/#match

spec.bs

jeremyroman · 2023-11-29T20:47:10Z

@yoshisatoyanagisawa @domenic still lgty both? I'll merge if so, as I think we've met the process requirements.

domenic · 2023-11-30T00:17:23Z

Still LGTM except I don't think you saw this comment:

You should also export the "match" definition https://urlpattern.spec.whatwg.org/#match

yoshisatoyanagisawa · 2023-11-30T01:19:12Z

Thanks for heads up.
Still lgtm.

jeremyroman · 2023-11-30T02:10:44Z

Still LGTM except I don't think you saw this comment:

You should also export the "match" definition https://urlpattern.spec.whatwg.org/#match

Whoops, super easy to miss unfortunately because of how GitHub requires me to switch to Files to be able to start a review (and not send tons of individual emails), which hides comments not attached to lines. Will fix.

…while routing rule registration, a=testonly Automatic update from web-platform-tests Add baseURL to the URLPattern condition while routing rule registration This behavior change was originally started in whatwg/urlpattern#182, and follows the spec change in whatwg/urlpattern#199. This CL changes the behavior of the router rule registration in the ServiceWorker Static Routing API, especially when the |urlPattern| condition receives URLPatternInit or USVString. Before this CL, the URLPatternInit input was accepted as it is, that means any unspecified fields are resulted in the wildcards (*). This behavior is inconsistent with the case when |urlPattern| accepts a string. When a string is passed, missing fields are complemented by baseURL, the SW script URL is internally treated as baseURL. After this CL, the URLPatternInit input also internally uses the SW script URL as a baseURL if it's not explicitly provided. This is achieved by the helper method `URLPattern::From()`, which was added in [1]. This change doesn't affect the case when the input is URLPattern, which means the input is the object constructed via `new URLPattern()`. [1] crrev.com/c/5053645 Bug: 1371756 Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039680 Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Commit-Queue: Shunya Shishido <sisidovski@chromium.org> Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org> Auto-Submit: Shunya Shishido <sisidovski@chromium.org> Cr-Commit-Position: refs/heads/main@{#1229724} -- wpt-commits: ffe06ef95d4d48be147e640fd9d1c489b810f929 wpt-pr: 43272

…while routing rule registration, a=testonly Automatic update from web-platform-tests Add baseURL to the URLPattern condition while routing rule registration This behavior change was originally started in whatwg/urlpattern#182, and follows the spec change in whatwg/urlpattern#199. This CL changes the behavior of the router rule registration in the ServiceWorker Static Routing API, especially when the |urlPattern| condition receives URLPatternInit or USVString. Before this CL, the URLPatternInit input was accepted as it is, that means any unspecified fields are resulted in the wildcards (*). This behavior is inconsistent with the case when |urlPattern| accepts a string. When a string is passed, missing fields are complemented by baseURL, the SW script URL is internally treated as baseURL. After this CL, the URLPatternInit input also internally uses the SW script URL as a baseURL if it's not explicitly provided. This is achieved by the helper method `URLPattern::From()`, which was added in [1]. This change doesn't affect the case when the input is URLPattern, which means the input is the object constructed via `new URLPattern()`. [1] crrev.com/c/5053645 Bug: 1371756 Change-Id: I5cce80fde05cf18237c8b6412b00e017ff5aad5b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5039680 Reviewed-by: Takashi Toyoshima <toyoshimchromium.org> Reviewed-by: Kouhei Ueno <kouheichromium.org> Commit-Queue: Shunya Shishido <sisidovskichromium.org> Reviewed-by: Yoshisato Yanagisawa <yyanagisawachromium.org> Auto-Submit: Shunya Shishido <sisidovskichromium.org> Cr-Commit-Position: refs/heads/main{#1229724} -- wpt-commits: ffe06ef95d4d48be147e640fd9d1c489b810f929 wpt-pr: 43272 UltraBlame original commit: 8cba5e465039e56ad23ca65c4e4b1c307f2f8379

jeremyroman requested review from sisidovski and domenic November 21, 2023 05:09

jeremyroman added 4 commits November 21, 2023 00:10

fix closing tag

9ce9724

create a separate enum for use of other APIs, to accept URLPattern di…

b04c982

…rectly

refer to URL absolutely

f8a6c47

fix refs

ea0cadb

domenic reviewed Nov 21, 2023

View reviewed changes

spec.bs Outdated Show resolved Hide resolved

jeremyroman added 2 commits November 21, 2023 01:44

misc review comments

759f2ae

defeat the evil trailing whitespace

a5c8450

sisidovski reviewed Nov 21, 2023

View reviewed changes

domenic reviewed Nov 21, 2023

View reviewed changes

spec.bs Outdated Show resolved Hide resolved

spec.bs Outdated Show resolved Hide resolved

spec.bs Outdated Show resolved Hide resolved

spec.bs Outdated Show resolved Hide resolved

spec.bs Outdated Show resolved Hide resolved

address review comments

262ed38

domenic approved these changes Nov 22, 2023

View reviewed changes

spec.bs Outdated Show resolved Hide resolved

domenic mentioned this pull request Nov 22, 2023

URLPattern usage WICG/compression-dictionary-transport#52

Closed

<cite>

2b2afdd

sisidovski approved these changes Nov 24, 2023

View reviewed changes

This was referenced Nov 24, 2023

URLPattern: Support hasRegExpGroups. denoland/rust-urlpattern#42

Closed

Support hasRegExpGroups kenchris/urlpattern-polyfill#125

Closed

Implement hasRegExpGroups kenchris/urlpattern-polyfill#126

Merged

yoshisatoyanagisawa reviewed Nov 27, 2023

View reviewed changes

chromium-wpt-export-bot mentioned this pull request Nov 27, 2023

Add baseURL to the URLPattern condition while routing rule registration web-platform-tests/wpt#43272

Merged

domenic reviewed Nov 29, 2023

View reviewed changes

spec.bs Outdated Show resolved Hide resolved

domenic reviewed Nov 29, 2023

View reviewed changes

spec.bs Outdated Show resolved Hide resolved

spec.bs Outdated Show resolved Hide resolved

domenic mentioned this pull request Nov 29, 2023

Revive of PR #1686. yoshisatoyanagisawa/ServiceWorker#1

Merged

review comments

c0c0204

jeremyroman mentioned this pull request Nov 29, 2023

URLPattern: hasRegExpGroups mdn/content#30640

Open

export match

0dd8dc0

jeremyroman merged commit 2c23b49 into whatwg:main Nov 30, 2023
2 checks passed

jeremyroman deleted the integrating branch November 30, 2023 02:16

yoshisatoyanagisawa added a commit to yoshisatoyanagisawa/ServiceWorker that referenced this pull request Nov 30, 2023

Introduce changes upon whatwg/urlpattern#199.

bdc2474

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a section on other specs integrating with URLPattern #199

Add a section on other specs integrating with URLPattern #199

jeremyroman commented Nov 21, 2023 •

edited by pr-preview bot

Loading

jeremyroman commented Nov 21, 2023 •

edited

Loading

domenic Nov 21, 2023

sisidovski Nov 21, 2023

jeremyroman Nov 21, 2023

domenic left a comment

jeremyroman commented Nov 21, 2023

domenic commented Nov 22, 2023

domenic left a comment

jeremyroman commented Nov 22, 2023

sisidovski left a comment

jeremyroman commented Nov 24, 2023 •

edited

Loading

yoshisatoyanagisawa Nov 27, 2023

jeremyroman Nov 27, 2023

yoshisatoyanagisawa Nov 30, 2023

yoshisatoyanagisawa Nov 27, 2023

jeremyroman Nov 27, 2023

yoshisatoyanagisawa Nov 28, 2023

jeremyroman Nov 29, 2023

yoshisatoyanagisawa Nov 27, 2023

jeremyroman Nov 27, 2023

yoshisatoyanagisawa Nov 30, 2023

yoshisatoyanagisawa commented Nov 27, 2023

domenic commented Nov 27, 2023

jeremyroman commented Nov 27, 2023

annevk commented Nov 28, 2023

jeremyroman commented Nov 28, 2023

annevk commented Nov 28, 2023

domenic left a comment

jeremyroman commented Nov 29, 2023

domenic commented Nov 30, 2023

yoshisatoyanagisawa commented Nov 30, 2023

jeremyroman commented Nov 30, 2023

Add a section on other specs integrating with URLPattern #199

Add a section on other specs integrating with URLPattern #199

Conversation

jeremyroman commented Nov 21, 2023 • edited by pr-preview bot Loading

jeremyroman commented Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic left a comment

Choose a reason for hiding this comment

jeremyroman commented Nov 21, 2023

domenic commented Nov 22, 2023

domenic left a comment

Choose a reason for hiding this comment

jeremyroman commented Nov 22, 2023

sisidovski left a comment

Choose a reason for hiding this comment

jeremyroman commented Nov 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoshisatoyanagisawa commented Nov 27, 2023

domenic commented Nov 27, 2023

jeremyroman commented Nov 27, 2023

annevk commented Nov 28, 2023

jeremyroman commented Nov 28, 2023

annevk commented Nov 28, 2023

domenic left a comment

Choose a reason for hiding this comment

jeremyroman commented Nov 29, 2023

domenic commented Nov 30, 2023

yoshisatoyanagisawa commented Nov 30, 2023

jeremyroman commented Nov 30, 2023

jeremyroman commented Nov 21, 2023 •

edited by pr-preview bot

Loading

jeremyroman commented Nov 21, 2023 •

edited

Loading

jeremyroman commented Nov 24, 2023 •

edited

Loading