[Phpmyadmin-devel] Column comments: First patch.

[Robin Johnson]

Hi Garvin,

Not trying to make your life difficult, but could you please:
1. Generate unified diffs with context instead of the RCS format.
  (use 'cvs -n diff -udHwbRN'). They are MUCH easier to review.
2. Seperate patches for each feature/todoitem/bugfix - this is to make
your submissions easier to review and add in incrementally. This is
partially important with touchy patches like your changes to
'libraries/display_tbl.lib.php3' that change a LOT of code scattered
about.
3. for your new strings, please go into the lang/ directory
and run:
"./add_message.sh '$strMessageName' 'Message contents'"
the single quotes are very important for this.
It will add your string to all of the files.
For this, do NOT use arrays like you are doing presently, sorry, but
they cause some problems with the translations system.
Then just attach a patch for the whole language directory as a single
patch.

Reviewing your code,
some possibly bugs:
libraries/build_dump.lib.php3:
-         if (PMA_MYSQL_INT_VERSION >= 32321) {
+         if (PMA_MYSQL_INT_VERSION >= 932321) {
typo ?

sql.php3:
+         list($usec, $sec) = explode(" ",microtime());
please use ' instead of " in your PHP code wherever expansion of
contents is NOT needed, it leads to a performance post and more
security.

libraries/common.lib.php3:
+       } elseif ($char == ';' and !empty($charbuff)) {
...
+       if ($append AND $i != strlen($string)) {
Is explicit use of 'AND' portable to PHP3 ? (Rabus?)
Possibly stick to '&&' ?

Other than those issues, your code looks good from a programming
standpoint and style standpoint. I haven't applied it to my tree yet,
because I'm busy on something else...

Write up some documentation once Michal has checked in his new
documentation layout, as I think this very important for the integrity
checks amongst other things. Doe the checks and so forth also work if
tables or columns are renamed or moved between databases?

Just make sure it conforms to the PEAR coding guidelines, and all should
be fine. (do NOT follow their suggestion about using indent on the
webpage, it destroys code if you aren't careful).
    
+1 from me on you getting a CVS account, yay or nay anybody else ?

-- 
Robin Hugh Johnson
E-Mail     : robbat2 at orbis-terrarum.net
Home Page  : http://www.orbis-terrarum.net/?l=people.robbat2
ICQ#       : 30269588 or 41961639
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 232 bytes
Desc: not available
URL: <http://lists.phpmyadmin.net/pipermail/developers/attachments/20030201/a63acdf2/attachment.sig>