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

Respect union wrap parameter as last argument #660

Merged
merged 2 commits into from Feb 9, 2015

Conversation

@werkt
Copy link

@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.

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
Copy link
Member

@bendrucker bendrucker commented Feb 3, 2015

Can you tack on a test with multiple unions?

@bendrucker
Copy link
Member

@bendrucker bendrucker commented Feb 3, 2015

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

@werkt werkt force-pushed the werkt:union_wrap_ignored branch from f6952c4 to 4bb23d2 Feb 4, 2015
Tests revealed some problems with argument and wrap interpretation,
corrected.
@werkt werkt force-pushed the werkt:union_wrap_ignored branch from 4bb23d2 to 16b234c Feb 4, 2015
@werkt
Copy link
Author

@werkt werkt commented Feb 4, 2015

Tests added.

@bendrucker
Copy link
Member

@bendrucker 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
Respect union wrap parameter as last argument
@tgriesser tgriesser merged commit d1d11f1 into knex:master Feb 9, 2015
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@tgriesser
Copy link
Member

@tgriesser tgriesser commented Feb 9, 2015

Great, thanks @werkt!

@khalilTN
Copy link

@khalilTN 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
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants