Hi Mojca, a couple of review comments inline below.
On Wed, Aug 10, 2016 at 11:09:30AM -0700, [email protected] wrote: > Revision: 151215 > https://trac.macports.org/changeset/151215 > Author: [email protected] > Date: 2016-08-10 11:09:29 -0700 (Wed, 10 Aug 2016) > Log Message: > ----------- > mp-buildbot: create a log file with progress of installed dependencies > > + # prepare the log file > + if ! [[ -d "${option_logdir}" ]] ; then > + mkdir -p "${option_logdir}" > + fi The double brackets aren't necessary here. Additionally, testing and then creating a directory has an implicit race condition (somebody else could create it in between), so it's good that you use -p here, but then you could just do mkdir -p directly and avoid the if completely. > + log_status_dependencies=${option_logdir}/dependencies-progress.txt > + # make ure to start with an empty file Typo, should be make *s*ure. > + echo -n "" > $log_status_dependencies Instead of explicitly printing nothing into a file, you can just run > "$log_status_dependencies" which does the same thing. Note that you should quote the redirect target, just in case $option_logdir contains spaces. This applies to all other uses of $log_status_dependencies as well. > + echo "" >> $log_status_dependencies echo "" is the same as just using echo without argument. > + text="Installing dependency ($dependencies_counter of > $dependencies_count) '${depname}', variants: '${depvariants}'" > + echo "----> ${text}" > + echo -n "${text}' ... " >> $log_status_dependencies You have an additional closing single quote after ${text} here, which leads to Installing dependency (41 of 205) 'xorg-xcb-proto', variants: '+python27'' ... [OK] Thanks for the work on this, though; it's much easier to read! -- Clemens _______________________________________________ macports-dev mailing list [email protected] https://lists.macosforge.org/mailman/listinfo/macports-dev
