Subject: Re: [PATCH 05/16] revision: add missing include

From: Junio C Hamano <[email-masked]>
Date: Thu, 31 Oct 2013 11:11:07 -0700

Felipe Contreras <[email-masked]> writes:

> Otherwise we might not have 'struct diff_options'.
>
> Signed-off-by: Felipe Contreras <[email-masked]>
> ---
>  revision.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/revision.h b/revision.h
> index e7f1d21..89132df 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -5,6 +5,7 @@
>  #include "grep.h"
>  #include "notes.h"
>  #include "commit.h"
> +#include "diff.h"
>  
>  #define SEEN		(1u<<0)
>  #define UNINTERESTING   (1u<<1)

This is a step in the right direction to change the contract between
this header file and its consumers, but I think it falls short of
doing a good job at it.

The rule used to be that "if you use a declaration in revision.h,
you must include diff.h before including it, even if you do not use
any declaration made in diff.h yourself". 

The new rule this patch introduces is "if you use a declaration in
foo.h, include foo.h, period---foo.h should handle its requirement
on its own internally and consumers should not have to care", which
is much saner.

But the patch needs to also remove '#include "diff.h"' from existing
consumers that themselves do not use any declaration from "diff.h"
(e.g. bundle.c; there are others), while keeping the inclusion in
those that do (e.g. builtin/commit.c). That can be a separate patch
that immediately follow this one, or a part of the same patch.

Thanks.