[Kde-extra-gear] [PATCH] cvsExtract.sh

[Klas Kalass]

Am Montag, 09. Februar 2004 19:52 schrieb Jeroen Wijnhout:
> [Kde-extra-gear] [PATCH] cvsExtract.sh
>
> Von:
> Jeroen Wijnhout <Jeroen.Wijnhout at kdemail.net>  (KDE)
>
>
> An:
> kde-extra-gear at kde.org
>
>
> Datum:
> Montag - 19:52:10
>
> Hi all,
>
> Here is a modified patch for cvsExtract.sh. It adds the possibility of
> splitting the package into an app and an i18n part ( --split,
> --noi18nbasetag ), creating the tarball ( --package ).
>
> It also prevents cvs from checking out empty directories.
>
> Ok to commit?

I noticed that you use pushd and popd - you can then remove the subshells I 
opened (the simple braces) where you introduced pushd/popd.

Nitpicking: For reading it would probably be nice if global variables were set
before they are used in function definitions. An example is 
CVS_CHECKOUT_OPTIONS_I18N_BASE.

By the way, that variable is the one I was referring to as not beeing 
correctly set in the other mail.

You only set it if USETAGFORI18NBASE is true, but the name of the variable
you replace is CVS_CHECKOUT_OPTIONS and not 'TAG_OR_BRANCH', so you should not 
leave it unset if simply no Tag is requested for I18N. The same goes for the 
admin tag btw. Why don't you set CVS_CHECKOUT_OPTIONS_* where 
CVS_CHECKOUT_OPTIONS  is set? Or introduce TAG_OR_BRANCH and use it instead 
or in addition to CVS_CHECKOUT_OPTIONS. That will avoid future 
misunderstandings.

And since I am already commenting on the patch: It would be nice if you could 
stick to the formatting/indentation used in the file. If you have strong 
feelings about the formatting style then please change it for the entire file 
in a seperate commit. I don't care what style is used, as long as it is only 
one :-)

Regards,
  Klas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: signature
Url : http://mail.kde.org/pipermail/kde-extra-gear/attachments/20040212/cd8b4092/attachment.pgp