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

Respect union wrap parameter as last argument #660

Merged
merged 2 commits into from Feb 9, 2015

Conversation

Projects
None yet
4 participants
@werkt

werkt commented Feb 3, 2015

union's wrap parameter was being ignored and misinterpreted when
building statements. Maintain the spirit of the original implementation
(variable number of callbacks) consistent with other instances of
variadic-like behavior (select/columns), including handling list of
callbacks in array, while still processing the wrap parameter.

Updated documentation which was missing a reference to wrap, and
provided a unit test for wrapped unions.

George Gensure
Respect union wrap parameter as last argument
union's wrap parameter was being ignored and misinterpreted when
building statements.  Maintain the spirit of the original implementation
(variable number of callbacks) consistent with other instances of
variadic-like behavior (select/columns), including handling list of
callbacks in array, while still processing the wrap parameter.
@bendrucker

This comment has been minimized.

Collaborator

bendrucker commented Feb 3, 2015

Can you tack on a test with multiple unions?

@bendrucker

This comment has been minimized.

Collaborator

bendrucker commented Feb 3, 2015

I know that was present before but it would be great to get it in now.

George Gensure
Adding multiple union tests for unwrapped/wrapped
Tests revealed some problems with argument and wrap interpretation,
corrected.
@werkt

This comment has been minimized.

werkt commented Feb 4, 2015

Tests added.

@bendrucker

This comment has been minimized.

Collaborator

bendrucker commented Feb 4, 2015

:) that's why tests are great. will review this this weekend.

tgriesser added a commit that referenced this pull request Feb 9, 2015

Merge pull request #660 from werkt/union_wrap_ignored
Respect union wrap parameter as last argument

@tgriesser tgriesser merged commit d1d11f1 into tgriesser:master Feb 9, 2015

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@tgriesser

This comment has been minimized.

Owner

tgriesser commented Feb 9, 2015

Great, thanks @werkt!

@khalilTN

This comment has been minimized.

khalilTN commented Mar 11, 2015

Hi, is this already available on npm ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment