-
Notifications
You must be signed in to change notification settings - Fork 571
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
GHA: test -DICINGA2_UNITY_BUILD=ON|OFF #9469
base: master
Are you sure you want to change the base?
Conversation
.github/workflows/unity.yml
Outdated
matrix: | ||
onoff: | ||
- ON | ||
- OFF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather call the workflow "build" and use different sets of compile flags in the matrix.
This comment is part of my secret plan (well, not so much anymore now) regarding #9411 (review), where this job could then serve as a basis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What for? For a use case which we don’t even agree on yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well it's not only about that. For now, the name "unity" describes the purpose of the job right now. Implementing my change would make it more flexible without drawbacks on the current use. You could also easily add more combinations of flags, like (non-)debug, (non-)systemd. Sure, not all combinations and not necessarily right now, but why not make the workflow easily extendable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want me to test (non-)debug, (non-)systemd?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't overload this check for now, we're putting enough stress on the CI already with our checks already.
We'll probably do some overhaul of the package jobs in the not so distant future, so let's see where this brings us and what extra jobs we might want then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forget it, there's also libstdc++, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you still insist on this? (Other blockers done, no add. matrix items in sight.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Insist on which part? I still think something like "build" would be a better name for the job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you insist on this? If yes, do you have suggestions for additional matrix vars (given we're not gonna replace our package build jobs yet)?
.github/workflows/unity.yml
Outdated
- name: Cancel previous jobs for the same PR | ||
if: "github.event_name == 'pull_request'" | ||
uses: styfle/cancel-workflow-action@89f242ee29e10c53a841bfe71cc0ce7b2f065abc | ||
with: | ||
workflow_id: deb.yml,docker.yml,raspbian.yml,rpm.yml,unity.yml,windows.yml | ||
access_token: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're reaching a point where I'm wondering if it would be possible to include this block from another file (in the same repo, making a separate action repo would be overkill).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please have a look at https://docs.github.com/en/actions/using-workflows/reusing-workflows, this should avoid having to duplicate the exact version of styfle/cancel-workflow-action
and workflow_id
in all workflow files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be solved with #9636
What about only the oldest and newest Boost versions with -in contrast to the existing GHA- only Debug build (on) and unity build off? |
to catch missing includes and duplicate static functions.
to catch missing includes and duplicate static functions.