Re: [RFC] Second parent for reverts

[Johan Herland <johan@herland.net>]

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

On Wednesday 09 May 2007, Shawn O. Pearce wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > On Wed, 9 May 2007, Daniel Barkalow wrote:
> > > The discussion about having a header to specify, for a revert
> > > commit, what it reverts made me realize that this header *would*
> > > be useful, but that we don't need a *new* header for it. I think
> > > that the right method is to add the parent of the reverted commit
> > > as a second parent for the revert.
> >
> > I am not so sure. In a sense, you are correct. But everybody who
> > does "git log --no-merges" would no longer see reverts. Which is
> > somewhat incorrect.
>
> Right.
>
> I've actually done what Daniel just talked about doing in one of my
> "production" repositories.  I did it by hand as a developer had
> created a bad merge and accidentially reverted 800 files during
> that merge.  80 or so commits later along a public non-rewinding
> branch coworkers realized things weren't right, and asked me
> to fix the mess.  As I wanted to save the blame data when I
> reverted-the-revert I did what Daniel suggests.
>
> But since the revert-the-revert wasn't really an interesting point
> in history, and neither was the bad merge, I don't really care that
> neither shows up with --no-merges.  The original bad merge was a
> simple honest mistake made by a developer who was new to Git, and
> was only caused because merge-recursive wasn't installed properly
> on that system.
>
>
> As Dscho says, most reverts are interesting points in time.  *Why*
> a particular revert was done is important.
>
> And so I have to disagree quite a bit with Daniel's idea, for exactly
> that reason.  If I'm looking at a block of code in a file I want to
> know why its there.  If blame tells me its a revert of something,
> that tells me we tried another path and it didn't work out.  I might
> be sitting here looking at this line because I'm thinking of redoing
> whatever it was that wasn't good!

Sorry for butting in with pretty much the same arguments as I had in the 
previous thread on the "Reverts" header field, but...:

Isn't this exactly why we could use the "Reverts" header field?

1. It would enable Daniel (and me) to get the "correct" blame data (for 
some version of "correct" that at least seems to make sense to us).

2. It would make it trivial for git-log (and other porcelains) to detect 
reverts and color them bright red in Shawn's and Dscho's logs, so that 
they can easily see when and why things were reverted.

3. It wouldn't screw up "git log --no-merges" since it doesn't interfere 
with the "real" parent data.

What am I still missing here?

> Hmm.  I should teach git-gui to parse out the revert message and
> let you click into its parent.  Simple enough.  Maybe it will be
> in 0.7.0.  Maybe it won't be.  ;-)

Isn't it better/cleaner to get this info from a (strict-format) header 
field, rather than parsing the commit message? (or worse, detecting 
reverse diffs/patches?)


Have fun!

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

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