Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > On Mon, Mar 1, 2021 at 3:50 AM Charvi Mendiratta <charvi077@xxxxxxxxx> wrote: >> t7500: add tests for --fixup=[amend|reword] options > > It's usually preferable for tests and documentation updates to be > bundled along with the patch which makes a particular change[1] rather > than waiting until the very end of the series and adding tests and > documentation covering all the changes made by patches earlier in the > series. As a reviewer, it is much harder to tell if the late-added > tests and documentation updates are comprehensive since it's difficult > to keep in mind all the changes made by earlier patches. > > When reading earlier patches in this series, I questioned whether or > not certain features of each patch were going to be covered by tests > or documentation updates, but I couldn't tell because those updates > weren't made at the same time as the change about which I was reading. > For instance, when reading the implementation of `--fixup:reword`, I > was wondering if the documentation was going to be updated to mention > that it would ignore changes staged in the index and leave the index > untouched, and I wondered if and hoped that tests would be added to > verify that the index was indeed left untouched. Over the course of > many patches, it can be difficult to keep track of all the accumulated > questions, which makes it onerous to review the final patches adding > the tests and documentation updates enmasse. > > I'm not necessarily suggesting that you re-roll merely to incorporate > the tests and documentation updates into the patches to which they > belong, but it's something to keep in mind for future submissions. > > FOOTNOTES > > [1]: Once in a while a patch introducing a change is so large on its > own that it may make sense to split tests and documentation updates > out to their own patches which immediately follow the patch to which > they apply, but that's different from delaying _all_ tests and > documentation updates and plopping them at the end of the series all > crammed together. A good piece of advice. Thanks.
