Rename supportedSources to knownSources #268
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
LGTM, but need more reviewers. |
rakuco
reviewed
Apr 26, 2024
index.html
Outdated
Comment on lines
750
to
752
| Each [=global object=] has an associated <dfn>frozen array of supported source types</dfn>, which is initialized | ||
| to the {{FrozenArray}} created from the sequence of strings values present in the enumeration | ||
| {{PressureSource}}, in alphabetical order. |
There was a problem hiding this comment.
I find this confusing, although I can see it's similar to https://w3c.github.io/performance-timeline/#dom-performanceobserver-supportedentrytypes:
- Why is the data per-global object if the list of known sources is not going to vary per global?
- Please mention in the commit message that the list is now mandated to be returned in alphabetical order, this was not the case before.
- "A FrozednArray created from PressureSource" does not really say that it contains only the source types this implementation might have support for.
What if you just say something like "each user agent has a list of PressureSource values corresponding to the source types that are known to the implementation" and the getter returns this list in alphabetical order?
There was a problem hiding this comment.
I guess I can say user agent instead of global object given they are stable across user agent
There was a problem hiding this comment.
Actually I think this is wrong. It's a FrozenArray (object) and that needs to be attached to a JS realm, thus global object
There was a problem hiding this comment.
The internal representation of the list of supported sources does not need to be a FrozenArray, it just needs to be represented as such when the attribute getter is accessed.
In other words, I think you could simply mention in https://www.w3.org/TR/compute-pressure/#internal-slot-definitions that the user agent has a list of supported sources and, in the getter here, say that it should return the list in alphabetical order. Since the getter returns a FrozenArray, the conversion between a list and a FrozenArray would be automatic.
|
I think we should merge this and we can make changes when we get answers to the files issue on the WebIDL repo. It is basically just a rename and it needs the global object to create a FrozenArray (which is an array and thus an object) |
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Apr 30, 2024
Based on specification change [1], supportedSources attribute is renamed to knowSources. [1]: w3c/compute-pressure#268 Bug: 337837870 Change-Id: Ic80d0c806531ed99f8bc2c368f89e55427b0202c
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Apr 30, 2024
Based on specification change [1], supportedSources attribute is renamed to knownSources. [1]: w3c/compute-pressure#268 Bug: 337837870 Change-Id: Ic80d0c806531ed99f8bc2c368f89e55427b0202c
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
Apr 30, 2024
Based on specification change [1], supportedSources attribute is renamed to knownSources. [1]: w3c/compute-pressure#268 Bug: 337837870 Change-Id: Ic80d0c806531ed99f8bc2c368f89e55427b0202c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5491092 Commit-Queue: Arnaud Mandy <arnaud.mandy@intel.com> Reviewed-by: Reilly Grant <reillyg@chromium.org> Reviewed-by: Mike Taylor <miketaylr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1294408}
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Apr 30, 2024
Based on specification change [1], supportedSources attribute is renamed to knownSources. [1]: w3c/compute-pressure#268 Bug: 337837870 Change-Id: Ic80d0c806531ed99f8bc2c368f89e55427b0202c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5491092 Commit-Queue: Arnaud Mandy <arnaud.mandy@intel.com> Reviewed-by: Reilly Grant <reillyg@chromium.org> Reviewed-by: Mike Taylor <miketaylr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1294408}
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Apr 30, 2024
Based on specification change [1], supportedSources attribute is renamed to knownSources. [1]: w3c/compute-pressure#268 Bug: 337837870 Change-Id: Ic80d0c806531ed99f8bc2c368f89e55427b0202c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5491092 Commit-Queue: Arnaud Mandy <arnaud.mandy@intel.com> Reviewed-by: Reilly Grant <reillyg@chromium.org> Reviewed-by: Mike Taylor <miketaylr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1294408}
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
May 3, 2024
Based on specification change [1], supportedSources attribute is renamed to knownSources. [1]: w3c/compute-pressure#268 (cherry picked from commit fc3b944) Bug: 337837870 Change-Id: Ic80d0c806531ed99f8bc2c368f89e55427b0202c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5491092 Commit-Queue: Arnaud Mandy <arnaud.mandy@intel.com> Reviewed-by: Reilly Grant <reillyg@chromium.org> Reviewed-by: Mike Taylor <miketaylr@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1294408} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5508494 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Arnaud Mandy <arnaud.mandy@intel.com> Reviewed-by: Raphael Kubo Da Costa <raphael.kubo.da.costa@intel.com> Cr-Commit-Position: refs/branch-heads/6422@{#626} Cr-Branched-From: 9012208-refs/heads/main@{#1287751}
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
May 6, 2024
…s to knownSources, a=testonly Automatic update from web-platform-tests compute pressure: Rename supportedSources to knownSources Based on specification change [1], supportedSources attribute is renamed to knownSources. [1]: w3c/compute-pressure#268 Bug: 337837870 Change-Id: Ic80d0c806531ed99f8bc2c368f89e55427b0202c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5491092 Commit-Queue: Arnaud Mandy <arnaud.mandy@intel.com> Reviewed-by: Reilly Grant <reillyg@chromium.org> Reviewed-by: Mike Taylor <miketaylr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1294408} -- wpt-commits: 02b375dd256fbe9090082cdda642b837b669404b wpt-pr: 45968
ErichDonGubler
pushed a commit
to erichdongubler-mozilla/firefox
that referenced
this pull request
May 10, 2024
…s to knownSources, a=testonly Automatic update from web-platform-tests compute pressure: Rename supportedSources to knownSources Based on specification change [1], supportedSources attribute is renamed to knownSources. [1]: w3c/compute-pressure#268 Bug: 337837870 Change-Id: Ic80d0c806531ed99f8bc2c368f89e55427b0202c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5491092 Commit-Queue: Arnaud Mandy <arnaud.mandy@intel.com> Reviewed-by: Reilly Grant <reillyg@chromium.org> Reviewed-by: Mike Taylor <miketaylr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1294408} -- wpt-commits: 02b375dd256fbe9090082cdda642b837b669404b wpt-pr: 45968
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The list is now mandated to be returned in alphabetical order, this was not the case before.
Fixes #266
Preview | Diff