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
