-
Notifications
You must be signed in to change notification settings - Fork 11
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
Draft CI requirements #61
base: main
Are you sure you want to change the base?
Conversation
|
||
**[CI.SCHEDULED_TEST]** RECOMMENDATION: CI enabled checks should run periodically across the entire repository. | ||
|
||
**[CI.SCEDULED_TEST_NOTIFY_FAILURE]** REQUIREMENT: There must be a established protocol to report CI failure (e.g. opening a GitHub issue) when scheduled CI fails. |
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.
Alternative wording: A CI run must not fail silently.
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 probably don't want an issue opened every time there's a failure...
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.
While it's very useful to know the state of success across compilers, I think it would be fine for say, AppleClang, to not be able to compile something because it's too old to have some feature that's needed, or even just desired, for the library component.
Requiring C++26 is OK. In fact it may be necessary for adoption of some proposals.
|
||
**[CI.TEST.CROSS_COMPILER_VERSIONS]** RECOMMENDATION: CI enabled tests should be executed across support major versions of appripriate compiler families. | ||
|
||
**[CI.DURATION_PER_COMMIT]** RECOMMENDATION: CI pipeline associated with commits should finish in a reasonable amount of time. |
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 we include a quantifiable "reasonable amount of time" as suggestion here?
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.
My recommendation is 10 mins for small projects, no longer than 30 mins.
|
||
## Continous Integration | ||
|
||
**[CI.PLATFORM]** REQUIREMENT: All CI should be implemented using GitHub Actions or an CI platform that appripriatly interacts with the code control system. |
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.
an CI platform that appropriately interacts with the code control system
may need better wording here. My intent here is to say "GitHub Actions is preferred, but if something checks PR, commits and write a check status to each commit and is reasonable, it is acceptable"
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.
If someone wants to pay for an external CI system, that might be acceptable? We can cross that bridge if and when.
But for beman standards purposes, configuration as code is important, and that code should live with the project. That's built in with GH Actions, but it does mean I can mostly coax Gitea to use them, too. I haven't tried Gitlab.
Internally, we use GHE, but not Actions, and instead use our own systems for CI and CD, but they're sufficiently agnostic that it's not an issue. It is why I prefer as minimal magic in the actions as possible, though, so it's straightforward to run tests on change.
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.
Yeah I agree, we need to come up with a wording for allowing acceptable external CI or make this item a recommendation.
**[CI.TEST.CROSS_PLATFORM]** REQUIREMENT: CI enabled tests must be executed across major platforms if appripriate. | ||
|
||
**[CI.TEST.CROSS_COMPILER_FAMILIES]** REQUIREMENT: CI enabled tests must be executed across major compiler families if appripriate. | ||
|
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.
These are listed as Requirements, but then come with the conditional 'if appropriate' -- which kinda makes them recommendations I'd say.
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.
This is fair.
My thinking here is, let's say we have a feature that relies on trunk version of gcc/ clang, this requirement could be waived.
Co-authored-by: Jeff Garland <[email protected]>
This PR drafts basic requirements for the use of Continuous Integration in beman project.
Note that I am not good with writing documentation so please scrutinize scrutinize scrutinize.
Most of the wordings here are more expressions of ideas than correctly worded standard.
Closes: #34 .
Discussion may concurrently happen on discourse.