> Matthew DeVore (5): > revision: invert meaning of the USER_GIVEN flag > list-objects-filter: implement filter only:commits > list-objects: store common func args in struct > list-objects: refactor to process_tree_contents > rev-list: handle missing tree objects properly Firstly, run every patch with "make DEVELOPER=1" - there is at least one "mixed declarations and code", which the Git coding style does not allow. I've already replied to patches 1, 2, and 5. Patches 3 and 4 look OK to me and seem like good changes (patch 4, in addition to reducing indentation, also reduces the scope of the local variables - so it is a good change). One last thing is that I'm not sure that this order of patches is the best order - in particular, if I run the tests at the 5th patch using a binary compiled at the 4th patch, I notice that cloning with "--filter=only:commits" fails with a cryptic error "fatal: bad tree object e891efadd67ca0c01b1c518a2fd91130d40f5904". This makes bisecting for errors difficult, but perhaps with this problem manifesting in only a few commits, it is not so bad. The ideal order is to put patches 3-5 before 1-2. I've tried the rearrangement myself and found many instances where I had to rewrite code because one patch introduces "ctx" and the other, NOT_USER_GIVEN. So as a reviewer, I'm on the fence about suggesting that the patches be reordered.
