style(clang-tidy): fix include headers clang-tidy errors #1688
Conversation
fixes sourcemeta#1684 Signed-off-by: Balakrishna Avulapati <bavulapati@gmail.com>
| @@ -1,10 +1,20 @@ | |||
| #include <sourcemeta/core/json_value.h> | |||
There was a problem hiding this comment.
Let's make modules only include public headers of other modules
| #include <sourcemeta/core/json_value.h> | |
| #include <sourcemeta/core/json.h> |
The idea is that only src/json can be internally aware that there is a json_value.h header, and every consumer should just require the public one
| @@ -1,11 +1,21 @@ | |||
| #include <sourcemeta/core/json_value.h> | |||
There was a problem hiding this comment.
| #include <sourcemeta/core/json_value.h> | |
| #include <sourcemeta/core/json.h> |
| @@ -1,5 +1,10 @@ | |||
| #include <sourcemeta/core/json.h> | |||
| #include <sourcemeta/core/json_value.h> | |||
There was a problem hiding this comment.
We shouldn't include json_value.h if we are including json.h
| @@ -1,4 +1,8 @@ | |||
| #include <sourcemeta/core/jsonschema.h> | |||
| #include <sourcemeta/core/jsonschema_types.h> | |||
There was a problem hiding this comment.
We shouldn't include jsonschema_types.h if we are including jsonschema.h
| @@ -1,9 +1,21 @@ | |||
| #include <sourcemeta/core/json_value.h> | |||
There was a problem hiding this comment.
| #include <sourcemeta/core/json_value.h> | |
| #include <sourcemeta/core/json.h> |
| @@ -1,10 +1,19 @@ | |||
| #include <sourcemeta/core/json_value.h> | |||
There was a problem hiding this comment.
| #include <sourcemeta/core/json_value.h> | |
| #include <sourcemeta/core/json.h> |
| @@ -1,7 +1,17 @@ | |||
| #include <sourcemeta/core/json_value.h> | |||
There was a problem hiding this comment.
| #include <sourcemeta/core/json_value.h> | |
| #include <sourcemeta/core/json.h> |
| #include <sourcemeta/core/json_error.h> | ||
| #include <sourcemeta/core/json_value.h> |
There was a problem hiding this comment.
We shouldn't include json_value.h (nor json_error.h as in the above line btw) if we are including json.h
| #include <sourcemeta/core/yaml.h> | ||
| #include <sourcemeta/core/yaml_error.h> |
There was a problem hiding this comment.
we shouldn't include yaml_error.h if we are including yaml.h
| @@ -1,11 +1,14 @@ | |||
| #include <sourcemeta/core/alterschema.h> | |||
|
|
|||
| #include <cassert> // assert | |||
| #include <sourcemeta/core/jsonschema_transform.h> | |||
There was a problem hiding this comment.
This file should only be including jsonschema.h
|
@jviotti I think we can use https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md I'm learning more |
|
I think we are looking for https://clangd.llvm.org/design/include-cleaner#umbrella-headers |
|
Not sure how googletest is using it. repo doesn't have any file containing |
|
I'm not sure, but sounds worth researching. In any case, just to keep it simple, let's aim to fix the ClangTidy warnings we get now, as of the latest changes. Once we have ClangTidy enforced in the |
fixes #1684