From: Pierre Habouzit Date: Tue, 08 Jul 2008 11:06:24 GMT Subject: Re: [PATCH 1/6] revisions: refactor init_revisions and setup_revisions. Message-ID: <20080708110624.GF19202@artemis.madism.org> In-Reply-To: On Tue, Jul 08, 2008 at 10:59:43AM +0000, Johannes Schindelin wrote: > 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