Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up order of includes #2015

Merged
merged 18 commits into from
Feb 7, 2023
Merged

Clean up order of includes #2015

merged 18 commits into from
Feb 7, 2023

Conversation

dthaler
Copy link
Collaborator

@dthaler dthaler commented Feb 2, 2023

Description

  • Put local headers before system headers for reasons explained in the updated DevelopmentGuide.md
  • Add clang-format rules to enforce header order. Note that clang 11 doesn't support case insensitive sorting of headers, only later versions do so headers are sorted case sensitively in the meantime.
  • Update file-level comment style in several files to match doxgen expectations.

Fixes #1963

Signed-off-by: Dave Thaler dthaler@microsoft.com

Testing

No impact.

Documentation

This PR includes doc updates.

Alan-Jowett
Alan-Jowett previously approved these changes Feb 2, 2023
@codecov
Copy link

codecov bot commented Feb 3, 2023

Codecov Report

Merging #2015 (b6a61ae) into main (c4f8f96) will increase coverage by 1.57%.
The diff coverage is 96.51%.

@@            Coverage Diff             @@
##             main    #2015      +/-   ##
==========================================
+ Coverage   82.41%   83.98%   +1.57%     
==========================================
  Files         141      149       +8     
  Lines       25402    27160    +1758     
==========================================
+ Hits        20935    22811    +1876     
+ Misses       4467     4349     -118     
Impacted Files Coverage Δ
ebpfapi/dllmain.cpp 70.00% <ø> (ø)
ebpfapi/rpc_client.cpp 56.14% <0.00%> (-1.01%) ⬇️
include/bpf2c.h 100.00% <ø> (ø)
include/ebpf_utilities.h 100.00% <ø> (ø)
libs/api/Verifier.cpp 83.42% <ø> (+1.60%) ⬆️
libs/api/ebpf_api.cpp 87.14% <ø> (+1.73%) ⬆️
libs/api/libbpf_system.cpp 100.00% <ø> (ø)
libs/api/windows_platform.cpp 95.83% <ø> (ø)
libs/api_common/api_common.hpp 86.48% <ø> (+2.70%) ⬆️
libs/api_common/map_descriptors.cpp 56.86% <ø> (ø)
... and 115 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Fixes microsoft#1963

Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
The generate script does not handle this file.

Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Dave Thaler <dthaler@microsoft.com>
@dthaler dthaler marked this pull request as ready for review February 4, 2023 00:03
#include <malloc.h>
#include <sddl.h>

#include "rpc_interface_s.c"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Since we have an exception here (that rpc_interface_s.c is included after svc_common.h), should we add a comment in the file mentioning this is deliberate?

Copy link
Collaborator Author

@dthaler dthaler Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not an exception per se, since this is including a C file not a header. DevelopmentGuide.md says:

DO include local headers (with "") before system headers (with <>).

Line 60 of .clang-format explicitly puts C files after headers.

@Alan-Jowett Alan-Jowett enabled auto-merge (squash) February 7, 2023 17:40
@Alan-Jowett Alan-Jowett merged commit 3d626ff into microsoft:main Feb 7, 2023
@dthaler dthaler deleted the header-order branch March 16, 2023 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix header file ordering in netebpext
4 participants