"W. Trevor King" <wking@xxxxxxxxxx> writes: >> I think it would be better to add initial tests with refactoring, and >> snapshot specific tests with snapshot support, e.g.: >> >> 1/2: gitweb: Refactor If-Modified-Since handling and add tests >> 2/2: gitweb: Add If-Modified-Since support for snapshots > > But the new tests would be for the new functionality (i.e. snapshot > support), so they wouldn't belong in the general refactoring commit. Then you are planning to split it in a wrong way. As I said, I do not think it matters that much for a small patch like this, but if the plan is to make the part to create i-m-s helper function as a standalone "refactoring" patch, then what Jakub outlined is the right way to go about it. In the first patch, you create i-m-s helper and update the existing code that can use the helper, without changing anything else. Do not touch snapshot code in this patch, if the current code does not support i-m-s in snapshot. And in the same patch, add tests for codepaths that use i-m-s to make sure your refactoring did not break them. In other words, if you remove the change to gitweb/ from the first patch and apply only the changes to the tests, the resulting new tests should pass with the current code that has i-m-s support inline without the i-m-s helper. And if you add back your change to gitweb/ for the refactoring, the test should still pass. And then in the second patch, you update the snapshot code and whatever else that can use i-m-s helper to support i-m-s. You can add tests to protect the new feature from future breakages in this patch. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html