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.
