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

'Array find from last' and 'Hashbang Grammar' moved to Stage 4 #1821

Merged
merged 2 commits into from Aug 26, 2022

Conversation

afmenez
Copy link
Contributor

@afmenez afmenez commented Jul 3, 2022

@ljharb
Copy link
Member

ljharb commented Jul 3, 2022

Let’s wait until the upstream PR is merged tho.

@zloirock
Copy link
Collaborator

zloirock commented Jul 3, 2022

Missed tests for typed arrays.

@afmenez afmenez changed the title 'Array find from last' moved to Stage 4 'Array find from last' and 'Hashbang Grammar' moved to Stage 4 Aug 23, 2022
@afmenez
Copy link
Contributor Author

afmenez commented Aug 23, 2022

There's already a "finished (stage 4)" category, would it be OK to move the features there until the upstream merge?

@ljharb
Copy link
Member

ljharb commented Aug 23, 2022

Both are now merged. I’m not sure we should bother having that category, since the window for stage 4+unmerged is rarely long.

Copy link
Collaborator

@zloirock zloirock left a comment

Choose a reason for hiding this comment

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

Missed tests for typed arrays.

@ljharb
Copy link
Member

ljharb commented Aug 26, 2022

@zloirock those weren't there before - why would we block updating the stage on adding missing tests?

@zloirock
Copy link
Collaborator

zloirock commented Aug 26, 2022

@ljharb they were missed in the early stages of this proposal. If it's moved to stable - such a significant component should not be missed. It can be added as copy-past with smoke testing for some minutes, so I have no ideas why it's still ignored.

@ljharb
Copy link
Member

ljharb commented Aug 26, 2022

I agree such a significant component shouldn't be missed - but that has no bearing on the currently incorrect stage information.

You are more than welcome to add them yourself, if it's so quick to write, but it would be highly inappropriate to demand that the author of this PR do something entirely unrelated to the purpose of the PR, I think.

As for "stable", stage 4 is no more stable than stage 3 - in both stages it's unlikely to change, but in both it could change, for the same reasons, so I think this is an incorrect characterization.

@ljharb
Copy link
Member

ljharb commented Aug 26, 2022

Given that this has two approvals, and the missing Typed Array tests are well out of scope for this PR, I'm going to merge this - that way someone can put up a separate PR to add the Typed Array tests without dealing with excessive merge conflicts.

@zloirock
Copy link
Collaborator

This proposal was changed on the way to stage 4 - typed array methods are one of those changes. They are missed in some environments - for example, in your polyfills.

Now I have no desire to add anything to this repository more than a review.

@ljharb ljharb merged commit dcc830d into compat-table:gh-pages Aug 26, 2022
@github-pages github-pages bot temporarily deployed to github-pages August 26, 2022 18:29 Inactive
@zloirock
Copy link
Collaborator

Awesome.

@ljharb
Copy link
Member

ljharb commented Aug 26, 2022

That's fine, since this repo didn't have any tests to begin with, nobody would be relying on anything that was changed along the way.

As for my polyfills, I simply don't have any Typed Array polyfills nor intend to (polyfilling something that's only a good idea to use for performance reasons doesn't make any sense), so nothing is missing from them.

@zloirock
Copy link
Collaborator

Let's remember why I stopped actively contributing to this project #1556 - and now you merged a PR with a changes request. Hypocrisy -)

Typed array constructors / logic and methods that are not related to performance are a few different things. However, you can think whatever you want - you have a strange opinion about many different things.

@zloirock
Copy link
Collaborator

BTW results that should be fixed by #1556 3 years ago are still wrong and Babel uses incorrect compat data for old Node versions.

@ljharb
Copy link
Member

ljharb commented Aug 26, 2022

This is all off topic for this PR. Manually verified data is welcome in another PR.

@zloirock
Copy link
Collaborator

No, your behavior is not off-topic - why did you merge it before fixes? -)

@ljharb
Copy link
Member

ljharb commented Aug 26, 2022

I explained why, and there's no need for further back-and-forth argument here.

Missing tests is not "broken" and doesn't need to be "fixed" in a PR that's not adding a feature.

@zloirock
Copy link
Collaborator

It's only your opinion - and I explained why it's needed. It's not only your repo - why do you think that the opinions of other maintainers make no sense?

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.

None yet

4 participants