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

Add collate for columns (MySQL) #1147

Merged
merged 3 commits into from
Feb 15, 2016
Merged

Add collate for columns (MySQL) #1147

merged 3 commits into from
Feb 15, 2016

Conversation

alexrsagen
Copy link
Contributor

This had been requested in #599 a year ago, but a PR was never created.

I have included a test that passed when i ran it.

This had been requested in #599 a year ago, but a PR was never created.

I have included a test that passed when i ran it.
@alexrsagen
Copy link
Contributor Author

@rhys-vdw Please have a look at this.

@meepen
Copy link

meepen commented Feb 11, 2016

very useful please add

@tjwebb
Copy link

tjwebb commented Feb 12, 2016

@alexrsagen can you add docs for this feature? Otherwise, lgtm

Also fixed the code example for "comment".
@alexrsagen
Copy link
Contributor Author

@tjwebb Is this okay? 2fae475

@alexrsagen
Copy link
Contributor Author

I have no idea how that one check failed, but i'm sure i didn't touch that, and the only thing i just changed was index.html... It said it timed out, so must be slow vm or old Node.js version causing that.

If you run the check again i'm sure it'd randomly decide to work...

@elhigu
Copy link
Member

elhigu commented Feb 13, 2016

I re-triggered the test :) it was just usual timeout when some test doesn't complete in 5000ms... Not sure if there is real problem in somewhere in our codebase or is it just random lagging of travis, which causes it. It happens maybe once in every 20 build or so.

@alexrsagen
Copy link
Contributor Author

@elhigu Great, thanks :)

I think the timeout issue is more than likely a problem with travis, as they create a very small VM or container for every test they run, so they can't use many resources.

Could you or another collaborator merge this please? Currently i'm using table.string("email").comment("' collate 'utf8_unicode_ci") to get past without the function i'm committing here :)

@elhigu
Copy link
Member

elhigu commented Feb 14, 2016

@alexrsagen code looks fine to me. We are trying to push out 0.10 so I would rather merge this after it is released... we hope to be able to make releases in faster cycle in future.

@alexrsagen
Copy link
Contributor Author

@elhigu Ah i see. Thanks for clarifying :)

elhigu added a commit that referenced this pull request Feb 15, 2016
Add collate for columns (MySQL)
@elhigu elhigu merged commit 96aa4d8 into knex:master Feb 15, 2016
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.

None yet

4 participants