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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux