Skip to content

core: replace header guards with #pragma once #21405

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

Merged
merged 1 commit into from
Apr 16, 2025

Conversation

JerelJr
Copy link
Contributor

@JerelJr JerelJr commented Apr 12, 2025

Contribution description

Updated header files in the core directory and its subdirectories by switching from header guards to #pragma once

Testing procedure

I used this script to find and correct header files that need to be changed. Change RIOT_PATH to the directory to be fixed

Issues/PRs references

See issue #21335

@JerelJr JerelJr requested a review from kaspar030 as a code owner April 12, 2025 22:18
@github-actions github-actions bot added the Area: core Area: RIOT kernel. Handle PRs marked with this with care! label Apr 12, 2025
@crasbe crasbe added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Process: needs >1 ACK Integration Process: This PR requires more than one ACK Process: missing approvals Integration Process: PR needs more ACKS (handled by action) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 12, 2025
@riot-ci
Copy link

riot-ci commented Apr 12, 2025

Murdock results

✔️ PASSED

04dafd5 core: replace header guards with #pragma once

Success Failures Total Runtime
10299 0 10299 11m:30s

Artifacts

@crasbe
Copy link
Contributor

crasbe commented Apr 14, 2025

Hello @JerelJr,

First all all welcome to the RIOT community and thank you for your first contribution. :)

It seems like the changes broke something, as the CI build does not complete anymore: https://ci.riot-os.org/details/9d47c2b748804fc4820232a5810c9d0e

Generally speaking, the core is the most critical part of RIOT and therefore requires special care when handling it, which is why I added the "needs >1 ACK" label.
Therefore it would've been better to start with an area with lower impact, but we'll see what we can do now.

Today the Hard Freeze will start and the Soft Freeze is already in place, so this is not a feature that could be merged before the next release: https://forum.riot-os.org/t/release-2025-04-soft-freeze-in-effect/4497/1

Perhaps you can look at the error in the CI system to try to locate which change broke the build in the meantime :)

@JerelJr
Copy link
Contributor Author

JerelJr commented Apr 14, 2025

Thank you for your feedback!

I started with core because it seemed like a relatively small amount of file changes, but I did not consider how critical that part of the code base is. I'll make sure to keep that in mind in the future. I will look into the CI output now to see what went wrong.

@maribu maribu changed the title Fix core headers core: use pragma once Apr 15, 2025
Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Would you mind squashing the commits into one?

core/lib/include/ringbuffer.h: removed trailing newline

core/lib/include/irq.h: restored header guard for irq.h to resolve build errors

core/lib/include/irq.h: added trailing newline to irq.h
@maribu maribu changed the title core: use pragma once core: replace header guards with #pragma once Apr 16, 2025
@maribu
Copy link
Member

maribu commented Apr 16, 2025

Since an agreement to use #pragma once has been reached in the weekly coordination, let's consider this an implicit 2nd ACK.

@maribu maribu removed Process: needs >1 ACK Integration Process: This PR requires more than one ACK Process: missing approvals Integration Process: PR needs more ACKS (handled by action) labels Apr 16, 2025
@maribu maribu enabled auto-merge April 16, 2025 05:30
@maribu maribu added this pull request to the merge queue Apr 16, 2025
Merged via the queue into RIOT-OS:master with commit 55f9d1c Apr 16, 2025
27 of 30 checks passed
@maribu
Copy link
Member

maribu commented Apr 16, 2025

Thx a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: core Area: RIOT kernel. Handle PRs marked with this with care! CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants