[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-extra-gear
Subject:    Re: [Kde-extra-gear] [PATCH] cvsExtract.sh
From:       Klas Kalass <klas.kalass () gmx ! de>
Date:       2004-02-12 22:45:02
Message-ID: 200402122345.07284.klas.kalass () gmx ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


Am Montag, 09. Februar 2004 19:52 schrieb Jeroen Wijnhout:
> [Kde-extra-gear] [PATCH] cvsExtract.sh
>
> Von:
> Jeroen Wijnhout <Jeroen.Wijnhout@kdemail.net>  (KDE)
>
>
> An:
> kde-extra-gear@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

[Attachment #5 (application/pgp-signature)]

_______________________________________________
Kde-extra-gear mailing list
Kde-extra-gear@kde.org
https://mail.kde.org/mailman/listinfo/kde-extra-gear


[prev in list] [next in list] [prev in thread] [next in thread]