style: use full paths for internal headers in capi/...#313
style: use full paths for internal headers in capi/...#313VDanielEdwards wants to merge 2 commits into
capi/...#313Conversation
|
Is there any particular reason you chose the google style guide? I have found that using the llvm style guide leads to headers being more self-contained. |
Signed-off-by: Daniel Edwards <Daniel.Edwards@vector.com>
d46ede3 to
dda8abe
Compare
No particular reason, it was just the first that came to mind. But I do like the ordering from local -> global as in the LLVM styleguide more than what the Google styleguide suggests. @MariusBgm would you object to switching the include-order to the LLVM styleguide? |
Signed-off-by: Daniel Edwards <Daniel.Edwards@vector.com>
snps-behrens
left a comment
There was a problem hiding this comment.
Looks pretty good. I did notice you removed pretty much all system includes from these files, even where there is extensive use of the standard library. Is that intentional?
| #include "silkit/participant/IParticipant.hpp" | ||
| #include "silkit/services/orchestration/all.hpp" | ||
|
|
||
| #include <map> |
There was a problem hiding this comment.
Why remove mutex here? It's still used in this file.
| #include <mutex> | ||
| #include <fstream> | ||
| #include "silkit/participant/IParticipant.hpp" | ||
|
|
There was a problem hiding this comment.
Why remove memory here?
| #include "capi/CapiImpl.hpp" | ||
| #include "config/ParticipantConfiguration.hpp" | ||
|
|
||
| #include "silkit/vendor/ISilKitRegistry.hpp" |
| @@ -6,12 +6,15 @@ | |||
| #define _CRT_SECURE_NO_WARNINGS | |||
There was a problem hiding this comment.
Not really in scope for this PR, but would it make sense to set this flag globally, rather than in every file where these warnings trigger (or have triggered in the past)?
e.g. by adding something like $<$<COMPILE_LANG_AND_ID:CXX,MSVC>:_CRT_SECURE_NO_WARNINGS> to the global add_compile_definitions.
Subject
Use full paths for internal headers in all sources and headers in
SilKit/source/capi/. Rearranges the includes to follow the guidelines in the LLVM Styleguide for C++Google Styleguide for C++. Removes unneccessary includes.Developer checklist (address before review)