Re: [PATCH] Make git revert warn the user when reverting a merge commit.

["Boyd Stephen Smith Jr." <bss@iguanasuicide.net>]

[-- Attachment #1: Type: text/plain, Size: 1789 bytes --]

Blah, my --in-reply-to didn't work so this didn't thread right.

On Thursday 2008 December 18 20:57:57 you wrote:
> On Thu, 18 Dec 2008, Boyd Stephen Smith Jr. wrote:
> > +		do {
> > +			switch (action) {
> > +			case REVERT:
> > +				warning("revert on a merge commit may not do what you expect.");
> > +				continue;
> > +			case CHERRY_PICK:
> > +				/* Cherry picking a merge doesn't merge the history, but
> > +				 * I don't think many people expect that.
> > +				 */
> > +				continue;
> > +			}
> > +			/* Unhandled enum member. */
> > +			die("Unknown action on a merge commit.");
> > +		} while (0);
> > +
>
> Wow.  That must be one of the, uhm, less beautiful ways to write
>
> 		if (action == REVERT)
> 			warning("revert on a merge commit may not do what you "
> 				"expect.");
> 		else if (action != CHERRY_PICK)
> 			die("Unknown action on a merge commit.");

My way, a smart compiler will warn at compile time that there's a new enum 
member that needs to be handled.  Your way, no such compile-time warning will 
be emitted.  At runtime, they have the same behavior.  Athestically, I agree 
with you, but my way may have technical advantages.

I did check the CodingGuidelines and didn't see this construct mentioned.

> Besides, I am actually pretty much against this change.

I've never had a need to revert a merge commit, so it's not a big win either 
way for me.  I wrote the patch because alan@clueserver.org had the revert 
behavior bite him and Linus suggested a warning might be apropos.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]