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

MSSQL removes support column width/length on data type int #2738

Merged
merged 7 commits into from Mar 5, 2019

Conversation

Projects
None yet
3 participants
@kamote
Copy link
Contributor

kamote commented Jul 30, 2018

  • fixes error Cannot specify a column width on data type int
MSSQL remove support column width/length on data type int
- fixes error `Cannot specify a column width on data type int`
- ignore length even if pass as agrument

@kamote kamote force-pushed the kamote:MSSQL/schema-int-fix branch from b6a461a to 053c412 Jul 30, 2018

@elhigu
Copy link
Collaborator

elhigu left a comment

Needs integration test.

@kamote

This comment has been minimized.

Copy link
Contributor Author

kamote commented Aug 5, 2018

@elhigu Most of current integration test are now using .integer() w/o length
https://github.com/tgriesser/knex/blob/master/test/integration/schema/index.js#L546-L547

@elhigu

This comment has been minimized.

Copy link
Collaborator

elhigu commented Aug 18, 2018

So if current integration tests are creating integers with specific width how come current integration tests are not throwing that error?

@kamote

This comment has been minimized.

Copy link
Contributor Author

kamote commented Aug 20, 2018

@elhigu upon looking, there is no MSSQL integration test that uses specific width which explains integration tests are not throwing.
In our case we found this kind of error since we are deploying same codebase with diff DB clients

@elhigu

This comment has been minimized.

Copy link
Collaborator

elhigu commented Aug 20, 2018

@kamote thanks for looking that up. In that case such test should be added (regression test to make sure the same thing will not start failing again).

@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Mar 4, 2019

@kamote Could you give me write access on your repo so that I could add test?

@elhigu

This comment has been minimized.

Copy link
Collaborator

elhigu commented Mar 4, 2019

@kibertoad did you try just to pull it and push changes? I have usually been able to do that... if it doesnt work, you can create your own PR also and link it here 👍

kibertoad added some commits Mar 4, 2019

integration tests added

@kibertoad kibertoad merged commit c2bf7a3 into tgriesser:master Mar 5, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.