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
