Page MenuHomePhabricator

Remove version check for mysql 4.1 from initEditCount.php and storage/fixBug20757.php
Closed, ResolvedPublic

Description

The scripts initEditCount.php and storage/fixBug20757.php has version check for a mysql version 4.1.0. MediaWiki needs at the moment 5.0.2 or later, so there is no need to do the check.


Version: 1.23.0
Severity: enhancement

Details

Reference
bz59126

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 2:36 AM
bzimport set Reference to bz59126.

Change 104713 had a related patch set uploaded by 01tonythomas:
Remove version check for mysql 4.1 from Maintenance scripts

https://gerrit.wikimedia.org/r/104713

We could probably just delete fixbug20757 most likely.

Change 104713 merged by jenkins-bot:
Remove version check for mysql 4.1 from Maintenance scripts

https://gerrit.wikimedia.org/r/104713

(In reply to comment #2)

We could probably just delete fixbug20757 most likely.

Is that necessary ? If you want, lets file a separate bug for that.

Bad fix in my opinion.

You have only removed one part of a AND condition, that means the condition itself was changed and not only dead code path removed.

For initEditCount.php:
You have changed the meaning for 'background mode' from 'LoadBalancer with more than one server OR mysql version < 4.1' to 'LoadBalancer with more than one server OR mysql'.

For storage/fixBug20757.php:
$lowerLeft is now a undefined variable for non-mysql server

(In reply to comment #5)

Bad fix in my opinion.
You have only removed one part of a AND condition, that means the condition
itself was changed and not only dead code path removed.
For initEditCount.php:
You have changed the meaning for 'background mode' from 'LoadBalancer with
more
than one server OR mysql version < 4.1' to 'LoadBalancer with more than one
server OR mysql'.

so it should be like 'LoadBalancer with more than one

server OR FALSE'. ?
For storage/fixBug20757.php:
$lowerLeft is now a undefined variable for non-mysql server

the else should be left untouched ?

For the first point: Yes, and a 'OR FALSE' is a no-operation, so you can remove that (The condition before was also a no-operation, because it always evaluated to false).

For the second point: You can also remove the outer if, because the else part maybe not working with other databases.