Code review comment for lp:~percona-dev/percona-server/release-5.1.53-12-slow_extended-fix

Revision history for this message
Alexey Kopytov (akopytov) wrote :

Oleg, please don't be lazy and create separate MPs for every change or bugfix you would like to push.

When reviewing a MP, I would like to see the following:

1) Description of the problem being fixed by the patch I'm looking at.
2) Description of the way the patch addresses the problem (if it's not obvious)
3) The patch itself.

What I see instead is a bunch of changesets like this:

--- cut ---
178. By Oleg Tsarev on 2010-12-17

    bzr merge lp:~percona-dev/percona-server/release-5.1.53-slow_extended
177. By Oleg Tsarev on 2010-12-17

    fix new line carrets
176. By Oleg Tsarev on 2010-12-17

    fix new line carrets
175. By Oleg Tsarev on 2010-12-17

    1) Improve tests for slow_extended
    2) Update infrastructure scripts
    3) Remove unnecessary test for show_slave_status_nolock.patch
    4) fix bug #691234
    5) fix bug #688643
--- cut ---

Then there is a bunch of changes in both code and test files. So I have to figure out which change correspond to which changeset *and* item from that changeset ("is this supposed to fix bug #691234 or bug #677643?"). Then I have to figure out the actual problem, because, believe it or not, a description like "Bug #668643 provocated by different update way for use_global_long_query_time" does NOT really describe anything. So I have to go read the bug report, figure out the problem, then look at your change again and make guesses about the proposed way to fix that problem.

This is _very_ time consuming. Please save me a few hours by just structuring your changes appropriately.

review: Disapprove

« Back to merge proposal