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

[Jeroen Wijnhout]

On Thursday 12 February 2004 23:45, Klas Kalass wrote:
> I noticed that you use pushd and popd - you can then remove the subshells I
> opened (the simple braces) where you introduced pushd/popd.

Done.

> 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

Ok, it is initialized now.

> 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.

Well, the CVS_CHECKOUT_OPTIONS is set in the switch statement that handles all 
the command-line options. If --usetag is comes before --noi18nbasetag on the 
command-line, then the CVS_CHECKOUT_OPTIONS_I18N_BASE variable is not set 
correctly. Therefore it is set after the switch statement.

> 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 :-)

I've noticed that both styles were used (i.e. spaces and tabs), so I decided 
to use my favorite style: tabs. I will convert my tabs to spaces. The other 
tabs can be converted to spaces in another occasion, since doing it now will 
obscure the diff.

If there are no objections I will commit the patch today.

best,
Jeroen

-- 
Kile - an Integrated LaTeX Environment for KDE
http://kile.sourceforge.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cvsExtract.sh.diff
Type: text/x-diff
Size: 9981 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-extra-gear/attachments/20040213/f5a4e5fb/cvsExtract.sh.bin