Skip to content

Conversation

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Apr 26, 2017

Downstream of missing commits from 17978 and 27441

@mention-bot
Copy link

@nickvergessen, thanks for your PR! By analyzing the history of the files in this pull request, we identified @MorrisJobke, @j-ed and @LukasReschke to be potential reviewers.

}
if (empty($tables)) {
$output->info('All tables already have the correct collation -> nothing to do');
}
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is a merge conflict mishap 😉 - let me remove it

@MorrisJobke MorrisJobke force-pushed the automatic-mysql-4byte-detection branch from daa7875 to 05faaa6 Compare April 27, 2017 02:31
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works 👍 (also the code makes sense)

@codecov
Copy link

codecov bot commented Apr 27, 2017

Codecov Report

Merging #4514 into master will increase coverage by 22.29%.
The diff coverage is 0%.

@@              Coverage Diff              @@
##             master    #4514       +/-   ##
=============================================
+ Coverage     31.37%   53.67%   +22.29%     
- Complexity    21898    22645      +747     
=============================================
  Files          1348     1361       +13     
  Lines         83372    86709     +3337     
  Branches       1334     1477      +143     
=============================================
+ Hits          26161    46537    +20376     
+ Misses        57211    40172    -17039
Impacted Files Coverage Δ Complexity Δ
config/config.sample.php 0% <ø> (ø) 0 <0> (ø) ⬇️
lib/private/Setup/MySQL.php 0% <0%> (ø) 6 <0> (+1) ⬆️
lib/private/DB/MySqlTools.php 0% <0%> (ø) 4 <4> (?)
core/Command/Db/ConvertMysqlToMB4.php 0% <0%> (ø) 5 <0> (ø) ⬇️
apps/comments/lib/EventHandler.php 79.16% <0%> (-8.34%) 7% <0%> (ø)
core/js/systemtags/systemtagsinputfield.js 80.08% <0%> (-1.1%) 0% <0%> (ø)
core/js/contactsmenu.js 91.7% <0%> (-0.21%) 0% <0%> (ø)
core/js/oc-dialogs.js 0.44% <0%> (-0.02%) 0% <0%> (ø)
core/js/files/client.js 87.6% <0%> (-0.02%) 0% <0%> (ø)
settings/templates/personal.php 0% <0%> (ø) 0% <0%> (ø) ⬇️
... and 393 more

@rullzer rullzer force-pushed the automatic-mysql-4byte-detection branch from eeeeb3b to 0aa5ddf Compare April 28, 2017 07:35
@MorrisJobke
Copy link
Member

@rullzer @blizzz @icewind1991 please review this one

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Changes make sense and code looks good. Just found a possible error in the config description.

* and MySQL can handle 4 byte characters instead of 3 byte characters.
*
* If you want to convert an existing 3-byte setup into a 4-byte setup please
* set the parameters in MySQL as mentioned below run the migration command:
Copy link
Member

Choose a reason for hiding this comment

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

"… set the parameters in MySQL as mentioned below and run the migration command: …"?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

Signed-off-by: Joas Schilling <[email protected]>
Copy link
Member

@LukasReschke LukasReschke left a comment

Choose a reason for hiding this comment

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

Makes sense 👍

@LukasReschke LukasReschke merged commit 4d101ca into master May 8, 2017
@LukasReschke LukasReschke deleted the automatic-mysql-4byte-detection branch May 8, 2017 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants