Toon Claes <toon@xxxxxxxxx> writes: > I was asked to provide test coverage for both code paths that produce this kind > of error message. Instead I've decided to ... Is code refactoring a substitute for making sure we have adequate test coverage? If not, "Instead I've decided to" is a strange thing to say at this point in the cover letter. Relative to the previous round, you cleaned up the code by introducing a helper function to show error messages, which gave us reduced duplication of the code. This is a very good thing and is worth mentioning. You however didn't get around to add adequate test coverage to the resulting code, which was one thing that the previous round was found lacking, and you can use help to improve these patches in this area, to help them get merged. Some of the error cases happen only in a corrupt repository that lacks objects that are expected to be there (e.g. a tree points at a blob in a non-lazy clone and blob is missing), so such a test may have to deliberately corrupt the test repository by removing .git/object/??/* files. Hopefully we'll hear from those who are willing to help. Thanks for working on this topic.
