On Fri, Aug 10, 2018 at 04:39:14PM -0700, Stefan Beller wrote: > > IMHO the whole for_each_*_object() interface should go in there (it even > > has packed_git defined there already!). I think I'd still just as soon > > do it on top of this series, but it might not be too bad to do as part > > of a re-roll. > > Yeah, I realize that I distracted myself and ranted about a different thing > other than the quality of this patch. (We had a couple of internal discussions > about project velocity and contributor happiness and I personally think this > derailing is some sort of anti pattern as fixing things like these is easy > as compared to user visible things such as file formats or configs. > Sorry for that.) It's a tough line to draw sometimes. This kind of ancillary discussion is often what spurs further work, so I think the discussions are good to have. And sometimes the right answer is "yeah, while we're here, let's clean this up, too". This may even be one of those cases. But sometimes the right answer is to push back a little and say "you're right, but let's deal with it later". And maybe later never even happens, but in that case maybe it wasn't that important in the first place. :) Or maybe it takes the same point coming up a few times to decide it's worth pursuing. I wish I had a good guideline for when to start such a discussion and when to push back. I mostly just follow my instincts, and my answer (on either side of that conversation) might change from day to day. I think the most important guideline is for everybody to be accepting of both sides of the conversation (i.e., it's OK to prod a little about ancillary issues as long as "yes, but not right now" is an acceptable answer). And then sometimes you catch me in a philosophical mood... -Peff
