From: Junio C Hamano Date: Mon, 29 Jul 2013 15:48:10 GMT Subject: Re: [PATCH] commit: correct advice about aborting a cherry-pick Message-ID: <7vli4p1i6d.fsf@alter.siamese.dyndns.org> In-Reply-To: <20130729152345.GA10393@sigill.intra.peff.net> Jeff King writes: > On Mon, Jul 29, 2013 at 08:18:45AM -0700, Junio C Hamano wrote: > >> > if (file_exists(git_path("MERGE_HEAD"))) >> > whence = FROM_MERGE; >> > - else if (file_exists(git_path("CHERRY_PICK_HEAD"))) >> > + else if (file_exists(git_path("CHERRY_PICK_HEAD"))) { >> > whence = FROM_CHERRY_PICK; >> > + if (file_exists(git_path("sequencer"))) >> > + sequencer_in_use = 1; >> >> Should this be >> >> sequencer_in_use = file_exists(...) >> >> so readers do not have to wonder what the variable is initialized >> to? > > Yeah, I think that is a readability improvement. I suppose the use-site > could also just run "file_exists" itself, but I wanted to keep the > filesystem-reading "magic name" bits together. I take that one back. The use-site is sufficiently far from this assignment that is protected with a cascading if that the reader needs to be aware that sequencer_in_use is initialized to zero anyway. The code you posted is just as readable, if not more. > I had also originally considered adding new states to "whence" to cover > these cases (i.e., FROM_CHERRY_PICK_MULTI), but there are fallouts all > over the place for call sites that do not care whether we are in the > single- or multi-commit mode. ;-)