Add review requirements for time-sensitive release updates#4746
Add review requirements for time-sensitive release updates#4746ogajduse wants to merge 1 commit intotheforeman:masterfrom
Conversation
github-actions
bot
added
Needs tech review
|
The PR preview for 3caaa6c is available at theforeman-foreman-documentation-preview-pr-4746.surge.sh No diff compared to the current base |
aneta-petrova
left a comment
aneta-petrova
left a comment
There was a problem hiding this comment.
Thanks for working on this @ogajduse! I have a couple of suggestions, which I'm hoping will help keep the guidelines as concise as possible.
CONTRIBUTING.md
Outdated
| Time-critical release changes may be merged without waiting 24 hours, but still require peer review and an ACK. | ||
| Non-trivial PRs might not only benefit from additional review but they also represent an opportunity for community members to ask questions and learn. | ||
| All PRs, including trivial ones, require peer review and an ACK before merging. | ||
| Substantive release notes containing Upgrade Warnings, Deprecations, and Headline Features require full review and should follow the standard 24-hour window. |
There was a problem hiding this comment.
| Substantive release notes containing Upgrade Warnings, Deprecations, and Headline Features require full review and should follow the standard 24-hour window. |
While true, I think you can safely drop this. Substantive always means non-trivial, which in turns means we must follow the standard process.
CONTRIBUTING.md
Outdated
| Examples of trivial PRs: Fixing a typo, fixing markup, fixing links, updating release version configuration files (e.g., `web/releases/*.json`), or adding fully generated release notes. | ||
| Time-critical release changes may be merged without waiting 24 hours, but still require peer review and an ACK. |
There was a problem hiding this comment.
| Examples of trivial PRs: Fixing a typo, fixing markup, fixing links, updating release version configuration files (e.g., `web/releases/*.json`), or adding fully generated release notes. | |
| Time-critical release changes may be merged without waiting 24 hours, but still require peer review and an ACK. | |
| Examples of trivial PRs: Fixing a typo, fixing markup, fixing links, updating release version configuration files (e.g., `web/releases/*.json`). | |
| Time-critical release changes, such as release notes, can be merged without waiting 24 hours, but still require peer review and an ACK. |
I would not consider "fully generated release notes" a trivial PR. However, we can consider them time-critical.
Add release configuration files and release notes to the list of trivial PRs that can be merged without the 24-hour waiting period. Based on discussion in theforeman#4707
ogajduse
force-pushed
the
rn-doc-guidelines
branch
from
086c338 to
3caaa6c
Compare
|
Thank you for your review, @aneta-petrova! I applied your suggestions. I wanted to clarify that patches introducing the "Upgrade warnings", "Deprecations", and "Headline features" sections in release notes I'm happy to refine the wording to distinguish between fully generated release notes (scripted Redmine updates for patch, RC2, and GA releases) that can be expedited and initial release notes (that we create right after Foreman branching with substantive sections) that should follow the standard 24-hour review. |
2 participants
What changes are you introducing?
Add release configuration files and fully generated release notes to the
list of trivial PRs that can be merged without the 24-hour waiting period.
Why are you introducing these changes? (Explanation, links to references, issues, etc.)
Based on discussion in #4707
Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)
Contributor checklists
Please cherry-pick my commits into: