git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/7] for_each_*_object: store flag definitions in a single location

From
Jeff King <peff@peff.net>
Date
Aug 11, 2018, 00:33 UTC
Message-ID
<20180811003319.GA29490@sigill.intra.peff.net>
In-Reply-To
<CAGZ79kZ0eN5eaBM8HjLVFpW8-ph3g0ym=-4ONTA7pgV=k3oz-Q@mail.gmail.com>
On Fri, Aug 10, 2018 at 04:39:14PM -0700, Stefan Beller wrote:
Show 11 quoted lines
> > 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
Previous: Stefan BellerNext: Jeff King
Message 7 of 27 in “speeding up cat-file by reordering object access”
  1. 0/7 speeding up cat-file by reordering object accessJeff King, Aug 10, 2018
  2. 1/7 for_each_*_object: store flag definitions in a single locationJeff King, Aug 10, 2018
  3. Stefan BellerAug 10, 2018
  4. Jeff KingAug 10, 2018
  5. Jeff KingAug 10, 2018
  6. Stefan BellerAug 10, 2018
  7. Jeff KingAug 11, 2018
  8. 2/7 for_each_*_object: take flag arguments as enumJeff King, Aug 10, 2018
  9. 3/7 for_each_*_object: give more comprehensive docstringsJeff King, Aug 10, 2018
  10. 4/7 for_each_packed_object: support iterating in pack-orderJeff King, Aug 10, 2018
  11. Derrick StoleeAug 15, 2018
  12. Jeff KingAug 16, 2018
  13. 5/7 t1006: test cat-file --batch-all-objects with duplicatesJeff King, Aug 10, 2018
  14. 6/7 cat-file: rename batch_{loose,packed}_object callbacksJeff King, Aug 10, 2018
  15. 7/7 cat-file: support "unordered" output for --batch-all-objectsJeff King, Aug 10, 2018
  16. Jonathan TanAug 13, 2018
  17. 0/4 finishing touches on jk/for-each-object-iterationJeff King, Aug 14, 2018
  18. 1/4 cat-file: use oidset check-and-insertJeff King, Aug 14, 2018
  19. 2/4 cat-file: split batch "buf" into two variablesJeff King, Aug 14, 2018
  20. 3/4 cat-file: use a single strbuf for all outputJeff King, Aug 14, 2018
  21. René ScharfeAug 14, 2018
  22. Jeff KingAug 14, 2018
  23. 4/4 for_each_*_object: move declarations to object-store.hJeff King, Aug 14, 2018
  24. Derrick StoleeAug 15, 2018
  25. Jeff KingAug 16, 2018
  26. Junio C HamanoAug 16, 2018
  27. Jeff KingAug 16, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.