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

Remove MySQL connection charset property defaults #1295

Conversation

tazmaniax
Copy link
Collaborator

Fixes

Fixes #1294

Purpose

Removes unneeded default MySQL character encoding and collation properties

Background Context

Latest Connector/J documentation recommends no character encoding and collation properties to be set by default so that these values to be set by the server while not precluding these properties from being overriden by the client if necessary

Copy link
Member

@xael-fry xael-fry left a comment

Choose a reason for hiding this comment

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

Tests are ko, can you have a look, thanks

@tazmaniax
Copy link
Collaborator Author

@xael-fry I had a look through the Travis log a couple of times and I don't think the issues had anything to do with the changes. When I compared the failed raw log with another successful log the issues seem to be more with resolving the dependency play-codeborne->guice 1.9. Could it just have been a bad run?
screenshot 2019-02-24 12 09 29

@asolntsev
Copy link
Contributor

@tazmaniax Yes, it seems it was just a bad run.
@xael-fry Can you restart the build? I see a warning that build was migrated from travis.org to travis.com ...

@tazmaniax
Copy link
Collaborator Author

tazmaniax commented Mar 1, 2019

For what its worth, I've been using this change in production since I submitted the pull request against MySQL 5.7

@tazmaniax
Copy link
Collaborator Author

@xael-fry can the build be restarted so we can see if the problem is repeated?

@xael-fry
Copy link
Member

@tazmaniax I cannot restart the build since travis move to the new url, can force the last commit to trigger the build again?

@tazmaniax
Copy link
Collaborator Author

@xael-fry sorry, is that something you want me to do? Can you clarify? Should I just make a dummy commit to force the build?

@xael-fry
Copy link
Member

Yes a dummy commit or a amend and force push. Thanks

@tazmaniax
Copy link
Collaborator Author

Horaah, the build worked :)

@xael-fry xael-fry merged commit 43de51c into playframework:master Mar 12, 2019
@xael-fry
Copy link
Member

@tazmaniax Thanks
Merged in master

@xael-fry xael-fry added this to the 1.5.3 milestone Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants