Web lists-archives.com

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




On 09.07.18 20:46, Jeff King wrote:
> 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