Skip to content

Conversation

@jsor
Copy link
Member

@jsor jsor commented Jul 6, 2015

This fixes a bug where some() never resolves if the input array contains not enough items.

some([1, 2, 3], 4)->then(function() {
    // Never called
});

@jsor
Copy link
Member Author

jsor commented Sep 7, 2015

ping @cboden @clue @WyriHaximus

Copy link
Member

Choose a reason for hiding this comment

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

The documentation seems to suggest otherwise. IMO it makes sense to reject this with a RangeException, but we should probably also add this special case to the README.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a note the the README (e29472f).

@clue
Copy link
Member

clue commented Sep 7, 2015

See remark about documentation for any(), otherwise LGTM 👍

@clue
Copy link
Member

clue commented Sep 8, 2015

README now LGTM :)

However, after further consideration, I think this should probably throw a subclass of LogicException rather than RuntimeException. After all this is in fact a logic error which could be detected at "compile time" (i.e. it does not really depend on a runtime decision). What's your view on this?

@jsor
Copy link
Member Author

jsor commented Sep 8, 2015

Good point. I've used RangeException after discovering this bug while looking through the bluebird code and extended the SPL RangeException (which in turn extends RuntimeException).

I think it would be strange to use the name RangeException and extend LogicException because this would differ from the SPL.

Maybe we should use another name...any suggestions?

README.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Would UnderflowException or LengthException, both of which extend LogicException, make sense here?

Edit: just realised that UnderflowException extends RuntimeException.

Copy link
Member Author

Choose a reason for hiding this comment

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

LengthException might be an option...

Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed to LengthException in cd6bb56.

@jsor jsor added this to the v3.0 milestone Jan 21, 2016
@jsor
Copy link
Member Author

jsor commented Jan 21, 2016

I'm planning to add this to 3.0.

@jsor jsor removed this from the v3.0 milestone Mar 25, 2016
@jsor
Copy link
Member Author

jsor commented Mar 25, 2016

Any objections against including this in a 2.4.0 release?

@jsor jsor merged commit 137f007 into master Mar 30, 2016
@jsor jsor deleted the some-underflow branch March 30, 2016 07:22
@cboden
Copy link
Member

cboden commented Mar 30, 2016

Technically isn't this an API break?

@jsor
Copy link
Member Author

jsor commented Mar 30, 2016

I think (and iirc we agreed on IRC) that this is a bug fix, because without this fix

some([1, 2, 3], 4)
    ->then(function() {
        // Never called
    })
    ->otherwise(function() {
        // Never called
    });

simply does nothing. It neither calls the fulfillment nor the rejection handler.

@cboden
Copy link
Member

cboden commented Mar 30, 2016

ah. 👍 for 2.4 then

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.

4 participants