[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: additions

From: Yavor Doganov
Subject: Re: additions
Date: Mon, 15 Feb 2010 12:57:00 +0200

В 21:19 +0200 на 14.02.2010 (нд), George Zarkadas написа:
> I have added some convenience targets to the www-el project's
>, that may be of value to other projects as well.

Thanks for these improvements, I hope to integrate them soon once we
agree on the details.

Some general comments:

      * There's no need to define variables for cp, wc, diff, etc. --
        these are standard utilities that must be in PATH on any
        Unix-like system.
      * We prefer to use $(RM) (which GNU make defines to `rm -f'
        internally) rather than plain `rm -f' as it's shorter and
        basically more visible.

More specifically:

>     format --> format all changed .po files to standard width

+       @echo Formatting .po files with msgcat : ; \
+       for file in $(translations) ; do \
+         wwwfile=$(wwwdir)`dirname $$file`/po/`basename $$file` ; \
+         if [ $$file -nt $$wwwfile ] ; then \
+           if [ `<$$file $(WC) -L` -gt 80 ] ; then \
+             $(MSGCAT) -o $$file $$file ; \
+             echo "  $${file}" ; \
+           fi ; \
+         fi ; \
+       done

Comparing the modification date of the two files is wrong, because there
are many valid situations when the file in www will be newer than
www-LANG (for example, when an English article is modified, the
article.LANG.po will be automatically updated in www, and thus will be
newer).  There is no ironclad way to determine which file is newer,
because the concept of "newer" is somewhat more complex here.  We could
eventually compare the headers PO-Revision-Date, but that won't work
when someone fixes an error in a .LANG.po in www.

Your supposed optimization when you check if the file is already wrapped
is rather unnecessary, and in fact serves against this purpose.
`msgcat' is very cheap/fast, and won't touch the file if there are no
actual modifications.  So in reality it's faster to run `msgcat' on all
files unconditionally rather than invoking `wc' together with `test'.

>     writeback --> copy changed .po files back to the local copy
>                        of the www repository

This is basically OK, but because of the reasons outlined above, has the
dangerous tendency of overwriting valid changes done in www.  I'll have
to think more about this...
At first glance, it looks like it would be fine to have this provided
that we document that the team leader must run `sync' before that, to
ensure that all eventual changes are merged in.  Maybe.

(I also do not like the name `writeback' as it is slightly confusing/not
sufficiently descriptive, but that's a minor point.)

>     clean --> get rid of backups and .mo files

Please report bugs to those editors that leave .mo files behind :-)
Files with ~ suffix are harmless as CVS ignores them.  Anyway, this
target is also OK, except this line which seems like unnecessary
obfuscation to me:

+         rm -f $${file/.po/.mo} ; \

Why not simply

        $(RM) $$


> Any comments or suggestions are welcomed, also.

These new targets need to be documented in the manual; I'll happily do

reply via email to

[Prev in Thread] Current Thread [Next in Thread]