Re: Rebase/cherry-picking idea

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 28, 2007, 09:47 UTC
Message-ID: <7v1waa7lcv.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <50645A3B-C5F0-4A99-A2B8-AD9251024244@wincent.com>

Wincent Colaiuta <win@wincent.com> writes:

Show 6 quoted lines

> The problem in this case was that my patch didn't receive any
> meaningful feedback (ie. suggestions for improvement), only a lot of
> bikeshed stuff about whether the environment variable should have an
> underscore prefix or not, whether or not I should use "export FOO=..."
> or not etc. So I didn't know what was necessary in order to get it
> accepted.

I do not think that is the case.

I think _GIT_FOO vs GIT_FOO is an important detail, not at all a bikeshed color, to make things consistent. "export VAR=VAL" also is a valid concern (you said in a separate message you only know about bash, and I later asked people if they use shells that get affected with it but we happily run otherwise. I personally do not think the latter is a problem, but since somebody already raised it as a potential issue, it gave us a good chance to hear from people on minority platforms, if only to build confidence in us to use that POSIX form.

Maybe it is just me, but I think my suggestion to replace not just "git commit" part but also "git add" part is also an important design issue.

In any case, after a discussion, sending out a readily applyable patch would help to make sure we reached a reasonable consensus; it makes it easier for other people to try out your proposed draft of the consensus without waiting for me or anybody else and give positive feedback.

The difference between the variant I posted and your revised one I see are:

 * As discussed, we have precedence like GITHEAD_* and GIT_REFLOG_ACTION
   that are used purely for inter-tool communication without the leading
   underscore, so I followed them for consistency.

 * The part of the message that is overridable is made wider; that way,
   we can later choose to have "git rebase --continue" do the final "git
   add -u" step, just like we made "git rebase --skip" to run "git reset
   --hard", without changing revert/cherry-pick,as I explained in the
   comment to my patch.

 * I made the help-message creation into a separate function.  This is
   just a minor detail, but I think it is good for readability.

 * I am setting and exporting the help environment near the beginning of
   rebase--interactive just once; again I think this is more readable.

But other than that, the basic idea is the same and is all yours.

If these details (I do not think the overridability is a minor detail, though) look like bikeshedding to you, that is somewhat sad. You seem to be very capable of producing usable code, but these details (consistency and flexibility) matter for longer term stability, and I would really want to see capable people pay attention to such details, especially I sometimes fail to do so myself.