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
Jeff King <peff@peff.net>
Date
Jul 9, 2018, 18:46 UTC
Message-ID
<20180709184632.GA9601@sigill.intra.peff.net>
In-Reply-To
<20180708144342.11922-3-dev+git@drbeat.li>
On Sun, Jul 08, 2018 at 04:43:38PM +0200, Beat Bolli wrote:
Show 9 quoted lines
> 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
Previous: Beat BolliNext: Beat Bolli
Message 9 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.