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

Add TC39 float16array #1530

Merged
merged 9 commits into from
Sep 11, 2024
Merged

Conversation

autonome
Copy link
Collaborator

@autonome autonome commented Jul 31, 2024

  • should manually go get the caniuse identifier when not in generated draft features?
  • is there a group this should be part of and where to verify? (i see in the burndown? can be generated then?)
  • is the baseline status right? baseline value of false doesn't seem to be documented in either the draft contributor guide nor the baseline doc linked there
  • how to handle global obj vs ctor? group them? (the burndown has separate entries)

@github-actions github-actions bot added the feature definition Creating or defining new features or groups of features. label Jul 31, 2024
@foolip
Copy link
Collaborator

foolip commented Jul 31, 2024

Hi @autonome, thanks for this PR!

My main piece of feedback here is that we've tended to avoid features that aren't supported by any browser yet, although we do have a few such high-profile features, such as Masonry and Temporal.

From https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Float16Array#browser_compatibility it looks like this is coming in Firefox 129. The data came from mdn/browser-compat-data#23730 which I think we can trust. But if we wait until next week we'll be able to see that everything is as expected, and avoid having to update the feature if some part didn't ship.

For the feature ID and filename, I'd suggest just float16array. The "tc39-" prefix is just from the spec shortname, and unnecessary in name and identifier.

@foolip
Copy link
Collaborator

foolip commented Jul 31, 2024

should manually go get the caniuse identifier when not in generated draft features?

Yes, search caniuse.com. In this case, there is no feature, so nothing to add.

is there a group this should be part of and where to verify? (i see in the burndown? can be generated then?)

Please use the typed-arrays group for this.

is the baseline status right? baseline value of false doesn't seem to be documented in either the draft contributor guide nor the baseline doc linked there

It is correct currently, because Firefox 129 hasn't been released yet. The value is documented here, and means it's not Baseline:

/** Whether the feature is Baseline (low substatus), Baseline (high substatus), or not (false) */

how to handle global obj vs ctor? group them? (the burndown has separate entries)

Include both, and more generally anything that is a part of the feature and shipped at the same time. The reason the constructor didn't show up in the spec draft here is because the spec link in BCD is to another spec:

https://github.com/mdn/browser-compat-data/blob/b04044b8628995deac3f0fcf42e3d36e28dd9cfc/javascript/builtins/Float16Array.json#L50

That's OK, but it's still part of Float16Array.

@autonome autonome marked this pull request as draft August 14, 2024 10:44
@Elchi3 Elchi3 mentioned this pull request Aug 15, 2024
@autonome autonome changed the title undraftify TC39 float16array Add TC39 float16array Aug 15, 2024
@ddbeck
Copy link
Collaborator

ddbeck commented Aug 15, 2024

No changes are needed here because of it, but I thought you'd be interested to know this came up in conversation on #1567 (review)

@Elchi3 Elchi3 mentioned this pull request Aug 20, 2024
features/float16array.yml Outdated Show resolved Hide resolved
@autonome autonome marked this pull request as ready for review September 11, 2024 10:00
@autonome autonome requested a review from Elchi3 September 11, 2024 10:00
@autonome
Copy link
Collaborator Author

@Elchi3 thanks! feel free to review or just fold into your ongoing JS work, i had just picked it up as an example early on.

Copy link
Collaborator

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Thanks @autonome! I think we're good to go with this one! 👍

@Elchi3 Elchi3 merged commit 0382da4 into web-platform-dx:main Sep 11, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature definition Creating or defining new features or groups of features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants