[RFC] CommitTemplate (was Re: [ANN bzr-vim] syntax and ftplugin for bzr_log.* files)

[Adeodato Simó]

* Aaron Bentley [Fri, 28 Jul 2006 09:31:14 -0400]:

Hi,

> Adeodato Simó wrote:
> > ------------------------------------------------------------------------

> > # Bazaar revision bundle v0.8

> > # message:
> > #   bzrlib/commit_template.py:
> > #     - New file.

> Our convention is that the first line of the commit message is a
> summary, and frequently the only line displayed.
> "bzrlib/commit_template.py:" does not make a good summary.

Yah, this slipped from my own particular commit message convention. In
any case, since this is in a branch that gets integrated later with a
proper first log line later, does it really matter? (it's okay if it
does).

> > @@ -0,0 +1,85 @@
> > +# Copyright (C) 2006 by Canonical Ltd
> > +
> > +# This program is free software; you can redistribute it and/or modify

> We have stopped using blank lines for vertical spacing in copyright
> statements.  This should be:

> @@ -0,0 +1,85 @@
> +# Copyright (C) 2006 by Canonical Ltd
> +#
> +# This program is free software; you can redistribute it and/or modify

> Feel free to fix up whatever file you copied it from.

A quick grep reveals at least 33 files suffer from that. If you'd like,
I can send a diff for all of them. Just let me know.

> >      def run(self, message=None, file=None, verbose=True, selected_list=None,
> > -            unchanged=False, strict=False, local=False):
> > +            unchanged=False, strict=False, template=None, local=False):

> We don't change the order of parameters on public methods, unless
> absolutely necessary.  Say it was being called like this:

>     cmd.run("mymessage", file, verbose, selected_list, unchanged,
> strict, local)

> Your change would break that usage.

Er, true. It was intended to go in the last place, of course. Only, I
was getting a very weird error, and I tried all sort of things,
including making the order of the arguments match the order of
self.takes_options, until I discovered the error was the fault of a
(IMHO) suboptimally implemented wrapper (*), commit_selector (**).
Then I forgot to revert it back.

> We use symbol_versioning.DEPRECATED_PARAMETER for this purpose, and we
> emit deprecation warnings when it's used.

> In order to be accepted, a patch like this would need to introduce tests
> for all the new code.

Of course, for both things. This was a RFC, not a merge request. Thanks
for the remainder, though. :)

So, from what it seems, the implementation would have to change
significantly, and generalize the commit message in another way. I'll
followup on this after I spend some time thinking about it.

Cheers,

Footnotes:

  (*) A wrapper that does:
    
      def run(self, arg1=default, arg1=default, arg1=default)

  instead of

      def run(self, arg_i_need1=default, arg_i_need2=default, **kwargs)


  (**) What I would like to see someday is `bzr commit --select-hunks`,
       a-la-darcs. Kinda like shelve-commit-unshelve. Do you think it'd
       be accepted if some day... an implementation appeared?

-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
                        Listening to: Hooverphonic - No More Sweet Music