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
Attachment:
signature.asc
Description: Digital signature