-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
tolength: remove superfluous +Infinity check #738
Conversation
The `+Infinity` check is superfluous as the `min` function takes care of that as well.
See #721 for last time... |
This looks like the same question as #721. |
With another group of people confused by this, I'll remove the redundant check. I will entertain arguments for a note, though I don't really want to go through the entire spec adding notes about +/- infinity for every usage of min/max. |
@thefourtheye congrats on your first contribution to ECMA262!! |
Thanks @bterlson :-) To be clear, you don't want to change other places like this in the spec? |
@thefourtheye I did a scan for other similar patterns w/ min/max and didn't see anything glaring. Which other places are you referring to? |
@bterlson Oh I didn't have anything else in mind. I just wanted to understand your sentence clearly. |
@thefourtheye Oh, my previous comment was saying that I could address the motivation to have the explicit +Infinity case by adding a note, but I can't convince myself that this usage of max/min is special enough to deserve the note and I don't want to add the note for every usage. Without a justification for ToLength being special I am ok not putting a note in anywhere. |
Oh okay, got it :-) Thanks @bterlson |
It should have been mine PR 😭 |
@zerkms I'm so sorry :( But, good news, there are boat loads of editorial issues you could fix! Just read a few clauses and I'm sure you'll find something. |
@bterlson I'm just kidding :-) Not a real problem at all :-) |
The
+Infinity
check is superfluous as themin
function takes careof that as well.