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

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

 



Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:

> Otherwise we might not have 'struct diff_options'.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> ---
>  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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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]