Code review comment for lp:~tsarev/percona-server/5.1_query_response_time_fix_bug_803867

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

* the patch adds a new file named response_time_distribution.patch.update. what is that for?

* we are going to move tests to the corresponding patches. so please change it so that all tests code to response_time_distribution.patch rather than a separate directory

* percona_query_response_time_show.inc: "\ No newline at end of file"

* percona_query_response_time_flush.inc: "\ No newline at end of file"

* too much conditional code in .inc files. It would actually be more simple and readable to move the _common_ code into .inc file(s) and let replication- and SP-related tests contain the rest.

* since most of the actual code changes occur in the response_time_distribution.patch.update which was apparently a mistake, I would like the above issues to be addressed before proceeding with review.

review: Needs Fixing

« Back to merge proposal