[Phpmyadmin-devel] Column comments: First patch.

[garvin_mailings at supergarv.de]

Hi Robin!

> 1. Generate unified diffs with context instead of the RCS format.
>   (use 'cvs -n diff -udHwbRN'). They are MUCH easier to review.

Yes, no problem for me. I didn't submit diff's yet, so I didn't know which
specific commands to use with.

> 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.

This will get kind of heavy because I will have to seperate the whole code
again...

> 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.

Allright, will do that.

> 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.

I just found out that I did submit the wrong diff, which was done from my
backup-directory instead of my latest working directory -- I diffed from the
dir, where I also made the first tries for the MIME-stuff, for which the
array-strings where for. I didn't look into the translation system yet and
would have noticed that a bit too late. So I can structure the whole thing a
bit different; even though arrays would be great for descriptions of each
transformation rule.

> libraries/build_dump.lib.php3:
> -         if (PMA_MYSQL_INT_VERSION >= 32321) {
> +         if (PMA_MYSQL_INT_VERSION >= 932321) {
> typo ?

Bug from my wrongly diffed working directory, yes. Shame on me. *darn*

> 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.

Allright - I took this snipped from the php.net website. Normaly I use '
throughout my code because of this reason. Thanks for noticing.

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

My own scripts normally all use "AND" and "OR" and so on, because I find
that easier to read. I tried to stick to your rules at most, but sometimes
it slipped through. I will take care on this for future patches.

> 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?

I will put documentation in that, as soon as I find out how to do that.
Haven't looked into it.

The checks do apply for moving/copying around databases, yes. Works pretty
smoothly. :-)

> 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).

My personal coding style (mostly the position of '}') is a bit different,
but yes, I will stick to them. :)

Thanks so far and have a nice week-end,
Garvin.