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

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:

quoted

+		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/                      \_/