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

enforce pragma once in header file #15264

Closed
lambdai opened this issue Mar 2, 2021 · 4 comments
Closed

enforce pragma once in header file #15264

lambdai opened this issue Mar 2, 2021 · 4 comments
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help! tech debt

Comments

@lambdai
Copy link
Contributor

lambdai commented Mar 2, 2021

I find my newly added header file not following the style guide Header guards should use #pragma once.

Looks like this is not unique

$ for f in `find include/ source/ test/ -name "*.h"`; do grep "pragma once" $f >/dev/null|| echo $f; done | wc -l
81

I will blindly add the pragma directive to make the future include no-brain.

Do we need a rule formatter to check/fix after the above change?

@lambdai lambdai added enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels Mar 2, 2021
@alyssawilk
Copy link
Contributor

yeah, looks like it's mostly but not all test code. It'd definitely be worth adding to the fix format script IMO.

@alyssawilk alyssawilk added tech debt and removed triage Issue requires triage labels Mar 2, 2021
alyssawilk pushed a commit that referenced this issue Mar 3, 2021
Commit Message:
Add pragma once almost every header.
Risk Level: LOW
Testing: self-contained
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
Part of #15264
[Optional Deprecated:]
[Optional API Considerations:]

Additional Description:

Signed-off-by: Yuchen Dai <[email protected]>
@lambdai
Copy link
Contributor Author

lambdai commented Mar 3, 2021

An quick fix is added formatted is not ready.

Also the exceptions are listed below for track

  1. chromium url
source/common/chromium_url/url_parse.h
source/common/chromium_url/url_parse_internal.h
source/common/chromium_url/url_canon_stdstring.h
source/common/chromium_url/url_canon_internal.h
source/common/chromium_url/url_canon.h
  1. test/config/integration/certs
test/config/integration/certs/clientcert_hash.h
...
  1. test/extensions/transport_sockets/tls/test_data/
test/extensions/transport_sockets/tls/test_data/

@lambdai lambdai added the help wanted Needs help! label Mar 3, 2021
@yanavlasov
Copy link
Contributor

The source/common/chromium_url should go away soon. I'm not quite certain when deprecation period has started for the code that uses it.

@botengyao
Copy link
Member

We can close this issue in favor of the merged PR above.

@phlax phlax closed this as completed Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help! tech debt
Projects
None yet
Development

No branches or pull requests

5 participants