Re: [PATCH 1/4] Add the ``--no-mount'' option.

[olafBuddenhagen]

Hi,

On Mon, Aug 10, 2009 at 04:13:38PM +0300, Sergiu Ivanov wrote:
> On Sat, Aug 08, 2009 at 04:53:29AM +0200, olafBuddenhagen@gmx.net wrote:
> > On Mon, Aug 03, 2009 at 10:39:51PM +0300, Sergiu Ivanov wrote:
> > > On Wed, Jul 29, 2009 at 09:27:10AM +0200, olafBuddenhagen@gmx.net wrote:
> > > > On Fri, Jul 17, 2009 at 01:56:57PM +0300, Sergiu Ivanov wrote:

> > > > > +/* Shows the mode in which the current instance of unionmount
> > > > > +   operates (transparent/non-transparent).  */
> > > > > +int transparent_mount = 1;
> > > > 
> > > > I think it would be clearer to default to "0" and set it on --mount...
> > > > 
> > > > But that's not terribly important really :-)
> > > 
> > > I set it to 1 because in this case argp_parse_common_options requires
> > > only the addition of the lines for handling the OPT_NOMOUNT option,
> > > which resumes to assigning 0 to transparent_mount.  Setting this
> > > variable to 0 initially will require adding one more line and several
> > > line shuffles.
> > 
> > Not at all. Unless I'm missing something crucial, you can use exactly
> > the same method -- only doing =1 in the OPT_MOUNT case instead of =0 for
> > OPT_NOMOUNT.
> > 
> > (Of course, you also have to switch the cases, as it's the other one
> > that needs a fallthrough when doing it this way around.)
> 
> This is what I meant when I was talking about ``line shuffling'' :-)

There is no shuffling really -- the new lines just need to be added in a
different place.

Most of the time, a good patch is one resulting in the cleanest (most
obvious) code. Keeping the patch itself simpler is not a goal, except in
bugfix releases.

-antrik-