Hi Phillip, Elijah & Junio, On Tue, 7 Apr 2020, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > > > Elijah Newren <newren@xxxxxxxxx> writes: > > > >> On Tue, Apr 7, 2020 at 7:11 AM Phillip Wood <phillip.wood123@xxxxxxxxx> wrote: > >>> > >>> From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> > >>> > >>> This reverts commit 4d924528d8bfe947abfc54ee9bd3892ab509c8cd. > >>> > >>> This is being reverted to enable some fixups for > >>> ra/rebase-i-more-options to be built on this commit. > >> > >> This makes sense to me, but it will be only the second 'Revert > >> "Revert..."' commit in all of git.git and I'm curious if Junio will be > >> unhappy with it. > > > > Nah, there isn't much to become unhappy about. > > > > I however suspect that the alternative would certainly be much nicer > > and easier to understand, which is to rebuild the 7-patch series > > c58ae96fc4..d82dfa7f5b but bugs already fixed, instead of doing this > > patch to take us back to a known buggy state and then fix the result > > with 5 more patches. Is that what you meant? > > After looking at the conflict resolution while merging the result of > applying these patches on top of the older codebase, I would have to > say that the approach """I've opted to add some cleanup commits on > top of Rohit's work rather than reworking his patches.""" may not > have been particularly a brilliant idea, not because the conflicts > arising from an older codebase are unpleasant to resolve (they seem > to be reasonably contained), but because it resurrects other > unwanted cruft we have cleaned up since then, and worse yet, it does > so without triggering conflicts. For example, we'll end up seeing > mentions of "'am' backend", which have all been updated to "'apply' > backend", in the documentation, and patches [2-6/6] do not fix them. > > [5/6] is an example of one more "unwanted" thing the reversion > resurrects that needed to be fixed, I guess? > > The result of applying all these patches and merging it to 'master' > and/or 'pu' may be more or less right, as far as the new features > added to the "rebase -i" by the series are concerned but there may > be many small "unwanted cruft" we may be resurrecting with [1/6], > so... I agree that it would make for a much nicer read if the entire patch series was simply rebased on top of v2.26.0, with drops instead of reverts. I suspect that 4/6 will not even become a fixup, but that the resulting patch is really more of an `Initial-patch-by: Rohit` material with Phillip as the author on record. As to the changes, I had a brief look over them, and I have nothing to add to Elijah's review except to stress how excited I am about the increased test coverage. From my perspective, this makes the patch series 10x better. Thanks, Dscho
