Review: Needs Fixing


Diff comments:

> === added file 'utils/merge_and_push_translations.sh'
> --- utils/merge_and_push_translations.sh      1970-01-01 00:00:00 +0000
> +++ utils/merge_and_push_translations.sh      2014-12-01 09:36:12 +0000
> @@ -0,0 +1,61 @@
> +## This script will fix translations in the translation branch

Add 

#!/bin/bash 

as first line and pushd and popd will work. I think they are the better 
solution.

> +## and then push the fix to the translations branch on Launchpad.
> +## Afterwards, the translations branch will be merged into trunk,
> +## the catalogs be updated and the result pushed to trunk on Launchpad.
> +
> +set -e # Exit as soon as any line in the bash script fails.
> +
> +# Move up if we're in the utils directory.
> +TEST=`(ls | grep "buildcat.py")`
> +if [ $TEST = "buildcat.py" ]; then

if [ -f "buildcat.py" ]; then gives the same result but actually checks that it 
is a file. I think you try to be too clever here, just demand that this script 
is run in the root directory. We do this already with other scripts. This just 
fills this one with logic that makes it more complicated but is not related to 
its task.

> +     echo "Leaving the utils dir"
> +     cd ..
> +fi
> +
> +echo "We are in the following directory:"
> +pwd
> +
> +# Make sure 'utils/buildcat.py' is there.
> +TEST=`(ls utils | grep "buildcat.py")`
> +if [ $TEST != "buildcat.py" ]; then

-f "utils/buildcat.py" && -f "more/files" should work. This might be useful 
http://tldp.org/LDP/abs/html/fto.html

> +     echo "Unable to find 'utils/buildcat.py'."
> +     echo "Make sure you start this script from Widelands' base or utils 
> directory.";
> +     exit;
> +fi
> +
> +# Make sure 'utils/remove_lf_in_translations.py' is there.
> +TEST=`(ls utils | grep "remove_lf_in_translations.py")`
> +if [ $TEST != "remove_lf_in_translations.py" ]; then
> +     echo "Unable to find 'utils/remove_lf_in_translations.py'."
> +     echo "Make sure you start this script from Widelands' base or utils 
> directory.";
> +     exit;
> +fi
> +
> +# Make sure we have the needed branches.
> +if [ ! -d "../trunk" ]; then
> +     echo "Please branch lp:widelands into ../trunk";
> +     exit;
> +fi
> +
> +if [ ! -d "../translations" ]; then
> +     echo "Please branch lp:~widelands-dev/widelands/translations into 
> ../translations";
> +     exit;
> +fi
> +
> +set -x # Print all commands.
> +
> +# Fix LF in translation branch.
> +cd ../translations && bzr pull
> +utils/remove_lf_in_translations.py
> +bzr commit -m "Fixed LF in translations." || true
> +bzr push lp:~widelands-dev/widelands/translations
> +
> +# Merge translations.
> +cd ../trunk && bzr pull
> +bzr merge lp:~widelands-dev/widelands/translations
> +bzr commit -m "Merged translations."
> +
> +# Update catalogues.
> +utils/buildcat.py
> +bzr commit -m "Updated catalogues."
> +bzr push lp:widelands
> 


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1219914/+merge/242772
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug-1219914.

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to