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

Re: [RFC PATCH 2/6] refs/refs-internal.h: avoid forward declaration of an enum

From
Beat Bolli <dev+git@drbeat.li>
Date
Jul 9, 2018, 19:30 UTC
Message-ID
<a3e6c187-74a5-1858-b207-b52d409b11be@drbeat.li>
In-Reply-To
<20180709184632.GA9601@sigill.intra.peff.net>
On 09.07.18 20:46, Jeff King wrote:
Show 30 quoted lines
> On Sun, Jul 08, 2018 at 04:43:38PM +0200, Beat Bolli wrote:
> 
>> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
>> index dd834314bd..a78b5cb803 100644
>> --- a/refs/refs-internal.h
>> +++ b/refs/refs-internal.h
>> @@ -1,6 +1,8 @@
>>  #ifndef REFS_REFS_INTERNAL_H
>>  #define REFS_REFS_INTERNAL_H
>>  
>> +#include "iterator.h"   /* for enum iterator_selection */
> 
> IMHO this kind of comment does more harm than good, because it is so
> prone to going stale (nobody is going to bother updating it when they
> add new dependencies on iterator.h). Anybody who is interested in the
> original reason can use "git blame" to dig up your commit message. And
> anybody who is thinking about deleting that line would need to dig into
> whether anything had been added in the meantime that also requires the
> include.
> 
> So at best it's redundant, and at worst it's slightly misleading. :)
> 
> Not worth a re-roll by itself, but it looked like you had a few other
> bits in the other patches to address.
> 
> Other than this minor quibble, the whole series looks good to me, modulo
> the existing review.
> 
> -Peff
> 
Ooosp, I've just sent the non-RFC reroll without this change.

Junio, would you squash this into [1/6] and [2/6], please (if you agree, of course :-)

Beat
Previous: Jeff KingNext: Jeff King
Message 10 of 39 in “Compile cleanly in pedantic mode”
  1. 0/6 Compile cleanly in pedantic modeBeat Bolli, Jul 8, 2018
  2. 4/6 sequencer.c: avoid empty statements at top levelBeat Bolli, Jul 8, 2018
  3. Eric SunshineJul 8, 2018
  4. Philip OakleyJul 8, 2018
  5. ig@drbeat.liJul 9, 2018
  6. Junio C HamanoJul 9, 2018
  7. Beat BolliJul 9, 2018
  8. 2/6 refs/refs-internal.h: avoid forward declaration of an enumBeat Bolli, Jul 8, 2018
  9. Jeff KingJul 9, 2018
  10. Beat BolliJul 9, 2018
  11. Jeff KingJul 10, 2018
  12. 3/6 convert.c: replace "\e" escapes with "\033".Beat Bolli, Jul 8, 2018
  13. 6/6 utf8.c: avoid char overflowBeat Bolli, Jul 8, 2018
  14. Johannes SchindelinJul 9, 2018
  15. Beat BolliJul 9, 2018
  16. Beat BolliJul 9, 2018
  17. Junio C HamanoJul 9, 2018
  18. Beat BolliJul 9, 2018
  19. Junio C HamanoJul 9, 2018
  20. Johannes SchindelinJul 9, 2018
  21. 1/6 connect.h: avoid forward declaration of an enumBeat Bolli, Jul 8, 2018
  22. 5/6 string-list.c: avoid conversion from void * to function pointerBeat Bolli, Jul 8, 2018
  23. Johannes SchindelinJul 9, 2018
  24. Junio C HamanoJul 9, 2018
  25. 0/6 Compile cleanly in pedantic modeBeat Bolli, Jul 9, 2018
  26. Beat BolliJul 9, 2018
  27. Junio C HamanoJul 9, 2018
  28. Beat BolliJul 9, 2018
  29. Beat BolliJul 10, 2018
  30. Junio C HamanoJul 11, 2018
  31. Johannes SchindelinJul 12, 2018
  32. Junio C HamanoJul 12, 2018
  33. 1/6 connect.h: avoid forward declaration of an enumBeat Bolli, Jul 9, 2018
  34. 5/6 string-list.c: avoid conversion from void * to function pointerBeat Bolli, Jul 9, 2018
  35. 3/6 convert.c: replace "\e" escapes with "\033".Beat Bolli, Jul 9, 2018
  36. 2/6 refs/refs-internal.h: avoid forward declaration of an enumBeat Bolli, Jul 9, 2018
  37. 6/6 utf8.c: avoid char overflowBeat Bolli, Jul 9, 2018
  38. 4/6 sequencer.c: avoid empty statements at top levelBeat Bolli, Jul 9, 2018
  39. Junio C HamanoJul 9, 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.