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