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 CIVICRM_TEMP_FORCE_UTF8; deprecate TempTable::setUtf8() #14004

Merged
merged 1 commit into from
May 9, 2019

Conversation

mfb
Copy link
Contributor

@mfb mfb commented Apr 8, 2019

Overview

This is a followup for #13658 to

  • Remove the CIVICRM_TEMP_FORCE_UTF8 constant; and
  • Deprecate the TempTable::setUtf8() method as tables should be assumed to use UTF-8 as default character set and collation (but may define columns to use some other character set and/or collation).

After

  • Setting CIVICRM_TEMP_FORCE_UTF8 would have no effect. All instances would be assumed to have UTF-8 as the default character set and collation for temporary tables.
  • TempTable::setUtf8(FALSE) could still be called, but this would be deprecated / discouraged, because the resulting default character set and collation is undefined (depends on the database default character set and collation). Tables that need to use a specific character set or collation should specify this in the column definition.

- Deprecate TempTable::setUtf8() as tables should be assumed to use UTF-8 as default character set and collation (but may define columns to use some other character set and/or collation).
@civibot
Copy link

civibot bot commented Apr 8, 2019

(Standard links)

@civibot civibot bot added the master label Apr 8, 2019
@eileenmcnaughton eileenmcnaughton added this to Main Review to-do-list in review board Apr 12, 2019
@mattwire
Copy link
Contributor

mattwire commented May 1, 2019

@eileenmcnaughton I'm happy with this but don't feel comfortable merging as I think it needs opinion from someone on #13658 - thoughts?

@eileenmcnaughton
Copy link
Contributor

@mattwire let's keep it for after 5.14 is forked. In 5.13 we make utf8 the default for all sites already - so this should be effectively 'no-change' - but the next cut is tomorrow & that will give it more rc time

@mattwire mattwire merged commit 6e46270 into civicrm:master May 9, 2019
review board automation moved this from Main Review to-do-list to done May 9, 2019
@mattwire
Copy link
Contributor

mattwire commented May 9, 2019

Merging now that 5.14 RC has been branched.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants