Code review comment for lp:~dandrader/frame/lp1080819

Revision history for this message
Stephen M. Webb (bregma) wrote :

I approve of most of these changes in general, with the following exceptions.

(1) as far as I know, it is not an error to mention a symbol name in a GNU linker version script and that symbol is not present. That means there is no need to run the preprocessor on the version script; a single version script should suffice regardless of package build options selected. Less to go wrong.

(2) You should avoid embedding device-specific control in the diagnostic output of configure. The right way to get pretty colour into the diagnostic output is to use the tput program. For example,

  bold_green=$(tput bold)$(tput setf 2)
  bold_white=$(tput bold)$(tput setf 7)
  reset=$(tput sgr0)

You should also use AC_MSG_NOTICE to send notices to the diagnostic output because (a) it send it to all the right places (echo only sends to stdout), and (2) the autoconf tool facilities react properly to command-line options like --quiet.

review: Needs Fixing

« Back to merge proposal