Hi All On 12/04/2020 18:47, Johannes Schindelin wrote: > 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 for all your feedback, I'll reroll as a new patch series based on master Best Wishes Phillip > Thanks, > Dscho >
