On Thu, Feb 2, 2023 at 10:51 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > On Thu, Feb 02, 2023, Vipin Sharma wrote: > > On Wed, Feb 1, 2023 at 3:24 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > I love the cleanup, but in the future, please don't squeeze KVM-wide changes in > > > the middle of an otherwise arch-specific series unless it's absolutely necessary. > > > I get why you added the macro before copy-pasting more code into a new test, but > > > the unfortunate side effect is that complicates grabbing the entire series. > > > > > > > Make sense. So what is preferable: > > 1. Make the big cleanup identified during a series as the last patches > > in that series? > > 2. Have two series and big cleanups rebased on top of the initial series? > > > > Or, both 1 & 2 are acceptable depending on the cleanup? > > 3. Post the cleanup independently, but make a note so that maintainers know > that there may be conflicts and/or missed cleanup opportunities. > > #1 is rarely going to be the best option. The big cleanup is going to necessitate > Cc'ing a lot of people that don't care about the base arch-specific changes, so > unless the base changes are one or two trivial patches, a lot of people end up > having to wade through a lot of noise. And aside from annoying people, that also > makes it more likely that someone will overlook the cleanup. > > As for #2 vs. #3, #3 is probably a better option in most cases. For broad cleanups, > odds are very good that there will be other conflicts beyond just the changes _you_ > have in-flight. E.g. in this case, any new tests and/or asserts that are in-flight, > sitting in other trees, etc., will suffer the same fate. I.e. whoever applies the > cleanup is going to need to resolve conflicts and/or look for other cleanup > opportunities anyways. For a scenario like this, a way to make life easy for the > maintainer applying the cleanup would be to provide a script, e.g. single grep > command, to look for potential cleanup spots. That communicates to the maintainer > that there may be silent "conflicts" and makes it easier for them to resolve such > conflicts. This is a good idea, to provide a grep or at least provide hints on how one has found places to edit. I will keep this in mind. Thanks > > Posting the cleanup separately means the two series/patches can proceed > independently, e.g. respinning one doesn't screw up the other, maintainers can > take the patches in whatever order they prefer, etc. > > There are undoubtedly exceptions, e.g. if the resulting conflicts are really nasty, > but those should be few and far between.
