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

Adding Temporal API #10643

Merged
merged 11 commits into from
Apr 26, 2022
Merged

Adding Temporal API #10643

merged 11 commits into from
Apr 26, 2022

Conversation

meyerweb
Copy link
Contributor

@meyerweb meyerweb commented May 26, 2021

Summary

Addition of the Temporal API to BCD.

@github-actions github-actions bot added the data:js 📟 Compat data for JS/ECMAScript features. https://developer.mozilla.org/docs/Web/JavaScript label May 26, 2021
meyerweb and others added 6 commits May 27, 2021 16:40
https://github.com/mdn/browser-compat-data/blob/master/schemas/compat-data.schema.json#L255-L264
doesn’t allow feature identifiers to contain parenthesis; instead it
restricts them to "^(?!__compat)[a-zA-Z_0-9-$@]*$".

And in the rest of the tree, existing feature identifiers method names omit the parenthesis.
The spec_url value isn’t allowed to be empty — so if there’s no spec URL
(yet), the spec_url key/value should be omitted completely.
spec_url values must contain a fragment ID.
I think this change also replaces tabs with spaces, and adds newlines to
the ends of the files.
@queengooborg
Copy link
Collaborator

This is currently blocked by #10619, where we're deciding whether or not to allow for all-false features within BCD. As such, I'll be marking this as "not ready" for the time being.

@queengooborg queengooborg added the not ready ⛔ This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. label Jul 14, 2021
@meyerweb
Copy link
Contributor Author

This is currently blocked by #10619, where we're deciding whether or not to allow for all-false features within BCD. As such, I'll be marking this as "not ready" for the time being.

Fair enough. Thanks for the update!

@meyerweb
Copy link
Contributor Author

meyerweb commented Oct 5, 2021

Temporal support is landing in Safari Technology Preview as of STP 133, so this is no longer all-false! If there’s a way to get this back on the “review and merge” train, let’s do it.

@meyerweb
Copy link
Contributor Author

meyerweb commented Oct 11, 2021

@queengooborg Could you remove the “Not ready” tag from this one, please? I’d do it myself but I don’t have the permissions.

@queengooborg queengooborg removed the not ready ⛔ This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. label Oct 11, 2021
@ddbeck
Copy link
Collaborator

ddbeck commented Jan 25, 2022

@meyerweb To get this moving again, we'd need to have the supported Safari features set to "version_added": "preview" (assuming it's not actually shipping yet). Do you want to start setting those? Maybe you know of some test suite, so you can quickly determine what, if any, parts of the API are unsupported (otherwise, we can talk about using the BCD collector to figure it out).

@meyerweb
Copy link
Contributor Author

@meyerweb To get this moving again, we'd need to have the supported Safari features set to "version_added": "preview" (assuming it's not actually shipping yet). Do you want to start setting those? Maybe you know of some test suite, so you can quickly determine what, if any, parts of the API are unsupported (otherwise, we can talk about using the BCD collector to figure it out).

I do. Remind me again, is there anything I need to say other than "version_added": "preview"? It’s been too long since i was in BCD source. (Pointers to documentation most welcome!)

@queengooborg
Copy link
Collaborator

If it's enabled by default in Safari TP releases, then "version_added": "preview" is all that you need!

@meyerweb
Copy link
Contributor Author

Updated the information to what my testing shows (using https://people.igalia.com/emeyer/js/tests/).

@queengooborg
Copy link
Collaborator

Thanks for the quick updates!

Last thing: I noticed that there are a number of added features marked as all-false. We should probably remove them until a decision has been made of whether to keep them or not, but I'll defer to @ddbeck on that one.

@meyerweb
Copy link
Contributor Author

Last thing: I noticed that there are a number of added features marked as all-false. We should probably remove them until a decision has been made of whether to keep them or not, but I'll defer to @ddbeck on that one.

Yes, portions of the Temporal API have yet to be implemented. My feeling is that if we have parts of the API, then we should have all of it, even if it’s all-false. That way readers get a clear view of what’s been done, and what hasn’t, instead of seeing that parts are available for testing out but not having any insight into the status of the rest of the API. But that’s my personal take and may run afoul of the current state of MDN policy.

@ddbeck ddbeck self-requested a review February 22, 2022 17:22
@queengooborg queengooborg removed the request for review from ddbeck April 19, 2022 06:14
Copy link
Collaborator

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

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

Thank you for waiting, and thank you for your hard work on this PR! Although we're still deciding whether we want to keep/introduce all-false features in BCD, the general consensus is currently to disallow features without a valid spec URL. You make a great point, in that if we're adding part of the API, we should add all of it.

This PR is looking good to me, thank you!

@queengooborg queengooborg merged commit 00d7b68 into mdn:main Apr 26, 2022
@meyerweb
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:js 📟 Compat data for JS/ECMAScript features. https://developer.mozilla.org/docs/Web/JavaScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants