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

Single header contains #include <flux/..> #221

Closed
matthew-8925 opened this issue Nov 26, 2024 · 2 comments · Fixed by #222
Closed

Single header contains #include <flux/..> #221

matthew-8925 opened this issue Nov 26, 2024 · 2 comments · Fixed by #222

Comments

@matthew-8925
Copy link

Hi,

FYI - the single include contains #include <flux/core/detail/jtckdint.h> - which makes it no longer a single file header and won't work on godbolt, etc.

#include <flux/core/detail/jtckdint.h>

Could be useful to add a precommit hook just grep'ing the single include file for #include "... or #include <flux/.., if this slips through often.

Thanks!

@tcbrindle
Copy link
Owner

Hi @matthew-8925, thanks for the bug report!

The problem is that the single header builder was only looking for files ending .hpp, so the new header ending .h got skipped. It should be fixed in #222.

Could be useful to add a precommit hook just grep'ing the single include file for #include "... or #include <flux/.., if this slips through often.

As far as I know this is the first time it's happened, and hopefully the last! I'd definitely consider a check if it happens again -- I guess the Github Actions job that builds the single header on commits to main would be the right place?

@matthew-8925
Copy link
Author

Thanks for the quick fix!

I see - not much point adding a grep check, doesn't seem like this exact issue could occur again.
An extra GH Action that runs a quick smoke test with the single include could make sense, as it should catch any issue where the single include doesn't compile.

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 a pull request may close this issue.

2 participants