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

Logic bug #5060

Closed
wants to merge 1 commit into from
Closed

Logic bug #5060

wants to merge 1 commit into from

Conversation

ssured
Copy link
Contributor

@ssured ssured commented Jun 23, 2014

If compare is a function which returns -1, 0 or 1, then 1-compare() has a result domain of 2, 1, 0, which is wrong. I did not hit this bug, just encountered it while reading the source

If compare is a function which returns `-1`, `0` or `1`, then `1-compare()` has a result domain of `2`, `1`, `0`, which is wrong. I did not hit this bug, just encountered it while reading the source
@rwjblue
Copy link
Member

rwjblue commented Jul 11, 2014

Looks correct to me, could you add test illustrating the problem and showing that this is fixed?

@rwjblue
Copy link
Member

rwjblue commented Jul 19, 2014

@ssured - Could you add a test to help make sure we don't accidentally regress?

@knownasilya
Copy link
Contributor

@ssured ping.

@stefanpenner
Copy link
Member

@knownasilya do you mind adding the test for him? Would love to see this in

@knownasilya
Copy link
Contributor

@stefanpenner sure, I'll submit a PR.

@stefanpenner
Copy link
Member

@knownasilya thank you kindly :)

@knownasilya knownasilya mentioned this pull request Aug 1, 2014
@wagenet
Copy link
Member

wagenet commented Aug 11, 2014

Closing in favor of PR.

@wagenet wagenet closed this Aug 11, 2014
@wagenet
Copy link
Member

wagenet commented Aug 11, 2014

Ah, actually the other PR is just tests it looks like.

@wagenet wagenet reopened this Aug 11, 2014
@stefanpenner
Copy link
Member

@lukemelia i think we noticed this the other day aswell

stefanpenner added a commit to stefanpenner/ember.js that referenced this pull request Nov 1, 2014
@lukemelia
Copy link
Member

This has been addressed by @stefanpenner's fix. Closing.

@lukemelia lukemelia closed this Dec 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants