Re: [PATCH v2 00/16] Convert parts of refs.c to struct object_id
From: brian m. carlson <hidden>
Date: 2016-06-15 23:04:38
On Sun, Apr 26, 2015 at 10:26:22PM +0200, Michael Haggerty wrote:
After that, there is one patch for each callsite, rewriting it to use for_each_ref natively (which usually entails modifying my_function to take an object_id parameter then undoing the wrapper). These patches involve a little bit of thought, but not too much. And the results are very bisectable because each patch makes a single small change. I also suspect it might be easier to rebase and/or merge my patch series, for the same reason. The end result was very similar to yours, so I am confident that the net result of your patch series is correct. But the remaining differences in the end results are also interesting. I made a few more changes in the neighborhood of the patches, not to mention a few formatting improvements in code that I touched. If you compare the tip of my branch, above, to the tip of yours (I uploaded that to my repo too, as branch "bc-oid-refs"), it may give you some ideas for other code that can be changed to object_id.
This is a very interesting approach. I've only just had time to look at it, but I like it. I agree that it's much more bisectable, although your series has a much larger quantity of patches. I feel like sending an 83-patch series to the list may frighten reviewers away. That's really the only thing preventing me from replacing my series with yours. Junio, what would you think about such a series? Would you prefer slightly larger patches, one per file, if it meant that we had many fewer patches, or would you prefer a large number of patches touching a single function each? -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
Attachments
- signature.asc [application/pgp-signature] 819 bytes