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

Normative: Remove implementation-defined behavior for Number methods #854

Merged
merged 1 commit into from
Apr 6, 2017

Conversation

littledan
Copy link
Member

Number.prototype.toFixed, toExponential and toPrecision previously
permitted a second argument, with implementation-defined behavior.
This language dates back to the original specification in ES3.
However, in testing with eshost and looking at the sources of V8,
SpiderMonkey, JSC and ChakraCore, I cannot find a usage of this
second argument.

This patch removes the implementation-defined behavior clause to
simplify spec reading and reduce a potential point of variability
and confusion among new implementations.

…hods

Number.prototype.toFixed, toExponential and toPrecision previously
permitted a second argument, with implementation-defined behavior.
This language dates back to the original specification in ES3.
However, in testing with eshost and looking at the sources of V8,
SpiderMonkey, JSC and ChakraCore, I cannot find a usage of this
second argument.

This patch removes the implementation-defined behavior clause to
simplify spec reading and reduce a potential point of variability
and confusion among new implementations.
@ljharb ljharb added needs consensus This needs committee consensus before it can be eligible to be merged. normative change Affects behavior required to correctly evaluate some ECMAScript source text labels Mar 21, 2017
@rwaldron
Copy link
Contributor

rwaldron commented Apr 6, 2017

This gained consensus on March 22nd

@leobalter leobalter removed the needs consensus This needs committee consensus before it can be eligible to be merged. label Apr 6, 2017
@ljharb ljharb added has consensus This has committee consensus. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Apr 6, 2017
@littledan
Copy link
Member Author

I don't think it's possible to write tests for this change.

Tangent: Once something has consensus, why not just write the label as "has consensus"--the awaiting merge part should be implicit, if the other appropriate conditions are met.

@ljharb ljharb removed the needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 label Apr 6, 2017
@ljharb
Copy link
Member

ljharb commented Apr 6, 2017

Sounds good; removed the tests label. My thinking was that it'd be more of an explicit call-to-action for @bterlson as he scans the PR list; I don't think it's important either way if the label has the CTA or not.

@leobalter
Copy link
Member

"has consensus" => "hey, Brian, please merge this" or just "Brian..."

both forms (short or long) works for me. As long we have a label, it's an explicit status after consensus is achieved.

@bakkot
Copy link
Contributor

bakkot commented Apr 6, 2017

"has consensus" might not mean "can be merged" if we're still getting tests ready, strictly speaking.

@bterlson
Copy link
Member

bterlson commented Apr 6, 2017

@bakkot I do not wait for tests to merge typically. Sorry for missing this PR.

@bterlson bterlson merged commit fbbbebb into tc39:master Apr 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus. normative change Affects behavior required to correctly evaluate some ECMAScript source text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants