Code review comment for lp:~qu1j0t3/maria/solaris10-port

Revision history for this message
Kristian Nielsen (knielsen) wrote :

Toby Thain <email address hidden> writes:

> Toby Thain has proposed merging lp:~qu1j0t3/maria/solaris10-port into lp:maria.

> Added build scripts for 32 bit x86 architecture on Solaris. Renamed some scripts for consistency. Changed to dynamic linking of libgcc.
> --
> https://code.launchpad.net/~qu1j0t3/maria/solaris10-port/+merge/6999
> You are requested to review the proposed merge of lp:~qu1j0t3/maria/solaris10-port into lp:maria.
>
> === modified file 'BUILD/compile-solaris-amd64'
> --- BUILD/compile-solaris-amd64 2009-05-09 04:01:53 +0000
> +++ BUILD/compile-solaris-amd64 2009-06-02 22:10:57 +0000
> @@ -26,7 +26,7 @@
> extra_flags="$amd64_cflags -D__sun -m64 -mtune=athlon64"
> extra_configs="$amd64_configs $max_configs --with-libevent"
>
> -LDFLAGS="-lmtmalloc -static-libgcc"
> +LDFLAGS="-lmtmalloc -R/usr/sfw/lib/64"
> export LDFLAGS
>
> . "$path/FINISH.sh"
>

I'm basically ok with these changes.

However, I would like you to add some explaining comments in the commit
message about why the changes are done, especially the above regarding
-static-libgcc and -R/usr/sfw/lib. Why are the changes needed, and what do
they do?

(generally it is more important in comments to explain _why_ than to explain
_what_; the code already shows what happens, but not why.)

Eg. if I had to merge these changes against conflicting changes from MySQL
upstream, I would have no clue about what to do to resolve the conflict.

You might also want to add some comments in the new script files for the Forte
C options if any of them are of special importance, it is up to you really,
you are the one who knows what they mean.

 - Kristian.

« Back to merge proposal