Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 02 Apr 2000 00:03:26 -0800
From:      Doug Barton <Doug@gorean.org>
To:        Alfred Perlstein <alfred@FreeBSD.ORG>
Cc:        cvs-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG
Subject:   Re: cvs commit: src/usr.sbin/mergemaster mergemaster.sh
Message-ID:  <38E6FECE.C52A3490@gorean.org>
References:  <200004020247.SAA93611@freefall.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
	I am heartily pissed by this commit. My objection to having mergemaster
imported into the tree was precisely this kind of situation. That's why
Bill Fumerola agreed to be the maintainer. 

	I reviewed the patches you sent me. Yes, it's taken me a long time to
review them and get back to you. I've been working a lot of hours
lately, and I haven't had much time for this. None of the changes you
proposed are urgent, your assertion regarding one-character input aside.
My priority for mergemaster changes are user-generated complaints (as I
explained to you via e-mail) and I have not received _one_  complaint
regarding any of the issues you raised. 

	I am going to send a patch tomorrow to bring the changes you made into
line with my design goals. Right now I'm too tired and too upset to give
it justice. If anyone has feedback on my comments below, please direct
it to -hackers or -current, since I don't want to abuse cvs-all and I'm
not on -committers. 

Thanks,

Doug

Alfred Perlstein wrote:
> 
> alfred      2000/04/01 18:47:21 PST
> 
>   Modified files:
>     usr.sbin/mergemaster mergemaster.sh
>   Log:
>   Make mergemaster only take single letters for options as well as loop
>   asking a question again if given an invalid input instead of assuming
>   what the user wants.  /etc is not the place to make assumptions when
>   given invalid input.

	The built in assumption is _always_ to do nothing. The only way a user
could get into trouble is with your convoluted example of i^H[Enter].
(The (i) option installs the new file into /etc, for those not familiar
with the program.) I specifically used (for example) i*) in my case
statements to allow for users who are used to typing words (like yes/no)
instead of insisting on one letter commands.  While I don't object to
the idea of a loop to handle user input in order to give the user
another chance to enter correct input, I don't like insisting on one
letter input, and I plan to reverse this change. 

>   Reformat some of the more convoluted code into seperate functions instead
>   of being inline using tabs instead of space indents.

	Thanks for the compliment. In both cases all your change did was move
code. The functions you created are only called once, so making
functions out of them is totally pointless. I prefer that the code be
"inline" since it's only used in one place. Making functions out of
single use code only serves to obfuscate it. It also makes it harder to
see the changes you actually made in the diff. I also don't like the tab
indenting, or I would have done it that way in the first place. Both of
these changes are going to be backed out by my patch. 

 
>   Allow the user to view merged files they created with sdiff.

	This is a pointless option, since if the user wants to view the file
before they install it, they can just leave it alone and not install it.
However, since it's already done it's probably harmless to leave it in. 
 
>   Allow the user to redisplay the diff between the installed file and
>   the new file again.

	Probably a good idea, I will probably leave it in. 
 
>   Time wasted waiting for review: 1 month 2 weeks

	Even if this had any relevance, you're not being accurate. Your first
letter was dated 3/1, to which I replied the next day. You sent me a
followup on 3/17 to which I replied the same day, indicating that I was
waiting to work on my version updgrade page first (which I personally
feel is much more important, timing wise) and which I'm still working on
when I have free time (which I don't have a lot of lately). Now I'll be
spending the time tomorrow that I had carved out to finish that to work
on this. Joy. 

	A cursory exam of the diff you committed shows other changes that you
didn't comment on. The mostly look harmless, but I'm going to give it
another look tomorrow. 

	On another note, several people have approached me about using a local
CVS repo if the user has one. After taking a look at that possibility
I'm not sure what advantages it would have. If someone can spell out
clearly to me what it is you want to accomplish, and hopefully provide
some sample commands as an illustration, I will take another look at it.
Private or public mail is fine. 

Doug
-- 
    "So, the cows were part of a dream that dreamed itself into
existence? Is that possible?" asked the student incredulously.
    The master simply replied, "Mu."


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?38E6FECE.C52A3490>