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

Set database charset utf-8 by default #8455

Merged
merged 1 commit into from Aug 14, 2015
Merged

Set database charset utf-8 by default #8455

merged 1 commit into from Aug 14, 2015

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Jul 29, 2015

fixes #6497

Made sure to have the charset set in DSN as well. It is done by Zend as well, meaning in the normal DB handler.

@tsteur tsteur added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Jul 29, 2015
@tsteur tsteur added this to the 2.15.0 milestone Jul 29, 2015
@mattab
Copy link
Member

mattab commented Aug 12, 2015

Maybe we could add an integration test that checks the charset is correctly utf8? (ci running on mysqli and pdo/mysql)

@tsteur
Copy link
Member Author

tsteur commented Aug 13, 2015

I'm not sure if it would be a very useful test as they run with utf8 by default probably?

@mattab
Copy link
Member

mattab commented Aug 13, 2015

maybe the test could change the charset to an exotic one, and check the exotic charset is set? that's optional anyway, just an idea

tsteur added a commit that referenced this pull request Aug 14, 2015
Set database charset utf-8 by default
@tsteur tsteur merged commit 03f6bc7 into master Aug 14, 2015
@tsteur tsteur deleted the 6497 branch August 14, 2015 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants