Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions.

From: Pierre Habouzit <madcoder@debian.org>
Date: Jul 8, 2008, 11:06 UTC
Message-ID: <20080708110624.GF19202@artemis.madism.org>
In-Reply-To: <alpine.DEB.1.00.0807081258010.4319@eeepc-johanness>

On Tue, Jul 08, 2008 at 10:59:43AM +0000, Johannes Schindelin wrote:

Show 9 quoted lines

> Hi,
> 
> On Tue, 8 Jul 2008, Pierre Habouzit wrote:
> 
> > setup_revisions has been split in two: parse_revisions that does 
> > (almost) only argument parsing, to be more like what parse-options can 
> > do, and setup_revisions that does the rest.
> 
> I do not see the sense of this change, except

  Well, it's required to remove "parse_revisions" at once if we one day
reach the goal of having only parse-opt based parsers.

> - blowing up your patch unnecessarily,

> - making it more inconvenient for users, and

  It makes it convenient for parse-opt based parser, and it makes it
convenient for people that don't have arguments to parse (and those are
many).

> - adding an opportunity for bugs to creep in.
> 
> In other words, I would like to see the semantics of setup_revisions() 
> untouched.

  FWIW like said I already submitted that patch under different versions
in the past, and I seem to recall that it was welcomed as it splits the
rev_info final fixups from the argument parsing which are quite
different steps.

  FWIW I really like the new semantics better. If you're worried about
the fact that people may be confused about the new semantics, fine,
let's rename it then.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org