[MERGE] New option --author for commit

[Lukáš Lalinský]

On Ut, 2007-08-07 at 14:09 -0400, Aaron Bentley wrote:
> Lukáš Lalinský wrote:
> > + * ``author`` - Author of the change, the value is in the same format as
>                                        ^ should be period.  You could
> argue for semi-colon, but the current text is a comma-splice.  Maybe
> "This value", would be better than "The value".
> 
> > + * ``branch-nick`` - Nick of the branch. It's either the directory name
>                             ^ "Nickname" is probably more helpful.
> 
> > + * ``bugs`` - A list of bug URLs and their statuses. The list is separated
> > +   by the new-line character (\n) and each entry is in format
> > +   '<URL> <status>'. Currently bzrlib uses only status 'fixed'.
>                                  ^ needs comma here

Thanks, fixed.

> > === modified file 'bzrlib/log.py'
> > --- bzrlib/log.py	2007-07-19 03:40:54 +0000
> > +++ bzrlib/log.py	2007-08-07 13:08:29 +0000
> > @@ -634,11 +634,14 @@
> >                  print >>to_file, indent+'parent:', parent_id
> >          print >>to_file, indent+'committer:', revision.rev.committer
> >  
> > -        try:
> > -            print >>to_file, indent+'branch nick: %s' % \
> > -                revision.rev.properties['branch-nick']
> > -        except KeyError:
> > -            pass
> > +        author = revision.rev.properties.get('author', None)
> > +        if author is not None:
> > +            print >>to_file, indent+'author:', author
> 
> This is improved, but you still need spaces around that '+'.

Fixed.

> > +        branch_nick = revision.rev.properties.get('branch-nick', None)
> > +        if branch_nick is not None:
> > +            print >>to_file, indent+'branch nick:', branch_nick
> 
> Oh, thanks.  (But again, '+' needs spaces)

I know I shouldn't do unrelated changed, but I couldn't resist changed
all these in log_revision.

> > === modified file 'bzrlib/mutabletree.py'
> > --- bzrlib/mutabletree.py	2007-07-04 18:24:24 +0000
> > +++ bzrlib/mutabletree.py	2007-08-07 13:08:29 +0000
> > @@ -201,6 +201,9 @@
> >              revprops = {}
> >          if not 'branch-nick' in revprops:
> >              revprops['branch-nick'] = self.branch.nick
> > +        author = kwargs.pop('author', None)
> > +        if not 'author' in revprops and author:
> > +            revprops['author'] = author
> 
> The explicit "if author is not None" is clearer than "if author".  It
> also distinguishes between None and ''.
> 
> This allows the author variable to supersede the ['author'] revprop.
> I'm not sure that's right.  Did you have a particular reason for that
> precedence?

I thought that since it modifies an user-supplied structure here, it
should never overwrite the original data. But the approach with raising
an exception makes more sense.

> Personally, I'm inclined to think that's a programming error, so I'd
> rather this:
> 
>         if author is not None:
>             assert author not in revprops
>             revprops['author'] = author
> 
> > === modified file 'bzrlib/tests/test_log.py'
> > --- bzrlib/tests/test_log.py	2007-07-05 00:29:29 +0000
> > +++ bzrlib/tests/test_log.py	2007-08-04 12:27:04 +0000
> > @@ -428,6 +428,35 @@
> >    simple log message
> >  """)
> >  
> > +    def test_author_in_log(self):
> > +        """Log includes the author name if it's set in
> > +        the revision properties
> > +        """
> > +        wt = self.make_branch_and_tree('.')
> > +        b = wt.branch
> > +        self.build_tree(['a'])
> > +        wt.add('a')
> > +        b.nick = 'test_author_log'
> > +        wt.commit(message='add a',
> > +                  timestamp=1132711707,
> > +                  timezone=36000,
> > +                  committer='Lorem Ipsum <test at example.com>',
> > +                  revprops={'author': 'John Doe <jdoe at example.com>'})
> 
> I think this should be updated to use the author parameter.

Fixed.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bzr-commit-author.diff
Type: text/x-patch
Size: 21529 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070807/64732aae/attachment-0001.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Toto je =?ISO-8859-1?Q?digit=E1lne?=
	=?ISO-8859-1?Q?_podp=EDsan=E1?= =?UTF-8?Q?_=C4=8Das=C5=A5?=
	=?ISO-8859-1?Q?_spr=E1vy?=
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070807/64732aae/attachment-0001.pgp