Code review comment for lp:~sergei.glushchenko/percona-xtrabackup/xb-tools

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

Sergei,

I didn't review the actual code, just want to comment on a number of basic things that caught my eye:

   - large blocks of empty lines, i.e. after ib_page_page_print_list()
     declaration (and what does that "SOME IBUF STUFF" comment mean?),
     and after ib_page_ibuf_rec_get_size()
   - lots of lines breaking the 80 chars limit
   - I see absolutely no reasons to use C++ and Boost in percona-redo.cc
     and percona-pprint.cc. Option parsing is available in my_getopt.c,
     which is already used in xtrabackup.c and xbstream.c. And
     pprint/reado are very small utilities.
   - large block of "#if 0"ed code in percona-redo.cc. If it's a
     debug-only code, create appropriately named #defines and some way
     to compile a debug binary. If that code is not supposed to be used
     by anyone, remove it.
   - some commented out code in percona-pprint.c. Same comments as in
     the previous item.
   - what are those "BEGIN LICENSE" / "END LICENSE" for?
   - can innodb_init_param() be moved to innodb_int.[ch] so we don't
     create separate files for it, which are also confusingly similar to
     innodb_int*?

review: Needs Fixing

« Back to merge proposal