Guido Günther <a...@sigxcpu.org> writes:

> Hi Daniel,
> This is almost good to go in, some minor things below:
>
> On Mon, Nov 21, 2011 at 03:44:41PM +0100, Daniel Dehennin wrote:
> [..snip..] 
>> -def fixup_trailer(repo, git_author, dch_options):
>> +def fixup_section(repo, git_author, options=[], dch_options=[]):
>
> It seems you always pass in all parameters so no need for default values
> here.

Right, and since I wrote that code I learn that it's a wrong thing to
use non-immutable as default values.


[...]

> Why can't you just do a getattr(options. opt)? The option should always
> be there (but the value might be None). It would also be nice to use a
> variable like: 
>      val = getattr(options, opt) 
>      if val:
>        ...
>        header_opts.append("--%s=%s" % (opt, val))
>
> instead of invoking getattr several times which is hard to read.

[...]

>> +        options = self.options[:]
>> +        options.append("--distribution=testing")
>> +        self.repo.create_tag("debian/0.9-1", msg="Pre stable release 
>> version 0.9-1", commit="HEAD~1")
>> +        ret = dch.main(options)
>> +        self.assertEqual(ret, 0)
>> +        lines = file("debian/changelog").readlines()
>
> This is almost the same in all tests. Could you move this into a common
> functtion to ease readability and reduce the amout of copy/paste code?
> Seems this only differs in the options passed, so this could be made the
> function argument. and the function could return the result of
> readlines()

I updated my tag[1] with your advices.

Thanks.

The following changes since commit 733573511a77bd2fdbbff61b9bd62b40ad63eac2:

  Move DscFile to separate module (2013-03-29 15:36:38 +0100)

are available in the git repository at:

  git://git.baby-gnu.net/git-buildpackage 
tags/dad/7335735/dch-configurable-changelog-entry

for you to fetch changes up to a5e6d3df6fdca7284c77f4d41e8922d0c6c93dc8:

  Add urgency management. (2013-03-31 14:50:18 +0200)

----------------------------------------------------------------
Rebased on latest experimental with Guido Günther advices.

Tests OK.

----------------------------------------------------------------
Daniel Dehennin (2):
      Add option to manage distribution fields for non snapshot mode.
      Add urgency management.

 docs/manpages/git-dch.sgml |   28 +++++
 gbp/scripts/dch.py         |   30 ++++-
 tests/11_test_dch_main.py  |  281 +++++++++++++++++++++++++++++---------------
 3 files changed, 237 insertions(+), 102 deletions(-)


Footnotes: 
[1]  
http://git.baby-gnu.net/gitweb/?p=git-buildpackage.git;a=shortlog;h=refs/tags/dad/7335735/dch-configurable-changelog-entry

-- 
Daniel Dehennin
Récupérer ma clef GPG:
gpg --keyserver pgp.mit.edu --recv-keys 0x7A6FE2DF

Attachment: pgpoxPwILpSkn.pgp
Description: PGP signature

Reply via email to