-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Eddy Petrișor wrote:
> Jari Aalto wrote:
>> tags 347322 + patch
>> thanks
> 
>> See attached patch to implement the requested feature.
>> The patch is against lates svn -r2262. 
> 
> Have you done functional tests?
> Did you test with and without the option you added?
> Did you test with options that might have broken (options available for
> pieces of code that use the new options)?
> 
>> Separate Changelog below:
> 
>> +  * svn-upgrade: Added option --verbose and --version.
>> +  * svn-buildpackage, svn-upgrade: Added option -N|--no-unrelease-tag to not
>> +    add word UNRELEASED to debian/changelog (Closes: #347322)
> 
> This is missing:
> * various cosmetic changes
> 
> I haven't checked the man pages, but I guess it will be trivial once we
> have discussed about functional aspects of the patch.
> 
> (I am eliminating them for now from the discussion.)
> 
>> === modified file 'svn-buildpackage'
>> --- svn-buildpackage 2007-01-28 11:34:02 +0000
>> +++ svn-buildpackage 2007-01-28 11:37:04 +0000
>> @@ -20,24 +20,25 @@
>>  repository must be in the format created by svn-inject, and this script
>>  must be executed from the work directory (trunk/package).
> 
>> -  -h, --help           Show the help message
>> -  --svn-dont-clean     Don't run debian/rules clean (default: clean first)
>> -  --svn-dont-purge     Don't wipe the build directory (default: purge after 
>> build)
>> -  --svn-no-links       Don't use file links (default: use links where 
>> possible)
>> -  --svn-ignore-new     Don't stop on svn conflicts or new/changed files
>> -  --svn-verbose        More verbose program output
>> -  --svn-builder CMD    Use CMD as build command instead of dpkg-buildpackage
>> -  --svn-override a=b   Override some config variable (comma separated list)
>> -  --svn-move           Move package files to .. after successful build
>> -  --svn-move-to XYZ    Move package files to XYZ, implies --svn-move
>> -  --svn-only-tag       Tags the current trunk directory without building
>> -  --svn-tag            Final build: Export && build && tag && dch -i
>> -  --svn-retag          Replace an existing tag directory if found while 
>> tagging
>> -  --svn-lintian        Run lintian after the build. s/lintian/linda/ to use 
>> linda
>> -  --svn-pkg PACKAGE    Specifies the package name
>> -  --svn-export         Just prepares the build directory and exits
>> -  --svn-reuse          Reuse an existing build directory, copy trunk over it
>> -  --svn-noninteractive Turn off interactive mode
>> +  -h, --help             Show the help message
>> +  -N, --no-unrelease-tag Do not add word UNRELEASED to debian/changelog
> 
> I'd say the option name choice is not quite inspired since it doesn't
> fall in line with the rest of the options.
> 
> Maybe "--svn-no-new-entry" or "--svn-dont-touch-log". Also, the place of
> the options would be better lower ...
> 
> Also I am not sure if this option deserves a short alternative while
> "--svn-only-tag" doesn't have one. Also, note that permanent
> configuration options, can be specified in ~/.svn-buildpackage.conf, so
> the size of the command line shouldn't be a problem for a person like
> the initial reporter.
> 
>> +  --svn-dont-clean       Don't run debian/rules clean (default: clean first)
>> +  --svn-dont-purge       Don't wipe the build directory (default: purge 
>> after build)
>> +  --svn-no-links         Don't use file links (default: use links where 
>> possible)
>> +  --svn-ignore-new       Don't stop on svn conflicts or new/changed files
>> +  --svn-verbose          More verbose program output
>> +  --svn-builder CMD      Use CMD as build command instead of 
>> dpkg-buildpackage
>> +  --svn-override a=b     Override some config variable (comma separated 
>> list)
>> +  --svn-move             move package files to .. after successful build
>> +  --svn-move-to XYZ      move package files to XYZ, implies --svn-move
>> +  --svn-only-tag         Tags the current trunk directory without building
> 
> ... around here.
> 
>> +  --svn-tag              Final build: Export && build && tag && dch -i
>> +  --svn-retag            replace an existing tag directory if found while 
>> tagging
>> +  --svn-lintian          Run lintian after the build. s/lintian/linda/ to 
>> use linda
>> +  --svn-pkg PACKAGE      Specifies the package name
>> +  --svn-export           Just prepares the build directory and exits
>> +  --svn-reuse            Reuse an existing build directory, copy trunk over 
>> it
>> +  --svn-noninteractive   Turn off interactive mode
> 
>>  If the debian directory has the mergeWithUpstream property, svn-buildpackage
>>  will extract .orig.tar.gz file first and add the Debian files to it.
>> @@ -67,6 +68,7 @@
>>  my @opt_override;
>>  my $opt_move;
>>  my $package;
>> +my $opt_no_unrelease_tag;
> 
>>  %options = (
>>     "h|help"                => \$opt_help,
>> @@ -98,7 +100,8 @@
>>     "svn-move-to=s"             => \$opt_move_to,
>>     "svn-builder=s"             => \$opt_buildcmd,
>>     "svn-override=s"             => [EMAIL PROTECTED],
>> -   "svn-pkg=s"             => \$package
>> +   "svn-pkg=s"             => \$package,
>> +   "N|no-unrelease-tag"    => \$opt_no_unrelease_tag
>>  );
> 
>>  use lib "/usr/share/svn-buildpackage";
>> @@ -190,7 +193,7 @@
>>     # no -d switch is there and no prebuild hook is set
>>     {
>>        if(  (!grep {$_ eq "-d"} @ARGV)
>> -         && (! withechoNoPrompt("dpkg-checkbuilddeps")) 
>> +         && (! withechoNoPrompt("dpkg-checkbuilddeps"))
>>           && ! $opt_prebuild
>>        )
> 
>> @@ -244,7 +247,10 @@
>>     system "$opt_pretag" if($opt_pretag);
>>     withecho ("svn", "-m", "$scriptname Tagging $package ($tagVersion)", 
>> "cp", $$c{"trunkUrl"}, $$c{"tagsUrl"}."/$tagVersion");
>>     system "$opt_posttag" if($opt_posttag);
>> -   withecho "dch -D UNRELEASED -i \"NOT RELEASED YET\"";
>> +
>> +   my $tag = $opt_no_unrelease_tag ? "" : "-D UNRELEASED";
>> +
>> +   withecho "dch $tag -i \"NOT RELEASED YET\"";
>>     print "\nI: Done! Last commit pending, please execute manually.\n";
>>     SDCommon::sd_exit 0;
>>  }
> 
> I think the changelog entry is a little misleading here:
> 
>> +  * svn-upgrade: Added option --verbose and --version.
> 
> 
>> === modified file 'svn-upgrade'
>> --- svn-upgrade      2007-01-28 11:34:02 +0000
>> +++ svn-upgrade      2007-01-28 11:37:04 +0000
>> @@ -17,13 +17,14 @@
>>  Upgrade a source code package from an upstream revision. The source code
>>  repository must be in the format created by svn-inject.
> 
>> -  -V, --upstreamversion  STRING    Forces a different upstream version 
>> string
>>    -c, --clean                      generic cleanup of upstream source - 
>> remove
>>                                     debian directory and object files
>>    -f, --force                      Force execution of certain operations
>> -  -v, --verbose                    More verbose program output
>> +  -N, --no-unrelease-tag           Do not add UNRELEASED word to 
>> debian/changelog
>>    -r, --replay-conflicting         Special cleanup action: replaces all
>>                                     conflicting files with upstream versions
>> +  -v, --verbose                    More verbose program output
>> +  -V, --upstreamversion  STRING    Forces a different upstream version 
>> string

> 
> 
> I'd rather say:
> 
> * reordered --verbose and --version options in help to appear towards
> the end (common practice)
> 
> Or am I missing something?

On second thought, no, this is not a good idea, the -V /
- --upstreamversion option is placed well at the top since is one of the
mostly used of the lot and does *not* have to do with the program's own
version.

I'll wait for comments and not commit yet, after all.

- --
Regards,
EddyP
=============================================
"Imagination is more important than knowledge" A.Einstein
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFFynt6Y8Chqv3NRNoRAoW3AJ0QvuHzyhT3QyzBanS56KnDN5mzCACgruBJ
yUcTem3msODzGaVazVU6+vA=
=O/tT
-----END PGP SIGNATURE-----


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to