docs: Start on the Release Process #1319
Conversation
ChristianTackeGSI
force-pushed
the
docs-contributing
branch
from
fe690e2
to
e986e81
Compare
|
Hm, how about just a single numbered list instead of heading per step? Additional steps that come to mind:
|
ChristianTackeGSI
force-pushed
the
docs-contributing
branch
from
e986e81
to
5d88460
Compare
Hmm. I don't care much. @karabowi added them in ChristianTackeGSI#1
Where would you suggest to put this?
done. |
That is usually, what I do in the context of a release among other things:
|
ChristianTackeGSI
force-pushed
the
docs-contributing
branch
from
5d88460
to
466ea8b
Compare
ChristianTackeGSI
force-pushed
the
docs-contributing
branch
from
466ea8b
to
8425225
Compare
|
@karabowi as you're currently in the process of creating a new release. Can you take another look at this? Maybe it would be good to merge and improve from there (if it's good enough for a merge)? If you want to take a look how the proposed changes look like when rendered: https://github.com/ChristianTackeGSI/FairRoot/blob/docs-contributing/CONTRIBUTING.md#user-content-creating-a-new-release |
ChristianTackeGSI
force-pushed
the
docs-contributing
branch
from
8425225
to
9abcbff
Compare
📝 WalkthroughWalkthroughThe Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (7)
CONTRIBUTING.md (7)
Line range hint
15-15: Consider using "first-time" instead of "first time" for grammatical correctness.
Line range hint
24-24: Simplify "at this point in time" to "now" for conciseness.
Line range hint
45-45: The phrase "will evolve over time" can be simplified to "will evolve" for conciseness.
Line range hint
65-65: Use "and" after 'both' to correct the usage error in "both in commit messages as well as in PR comments".- both in commit messages as well as in PR comments if applicable. + both in commit messages and in PR comments if applicable.
Line range hint
76-77: Move "Rarely" to a position after the subject for better readability and correct the verb form of "opt-out".- Rarely, it is needed to opt-out of *clang-format* for certain code sections + It is rarely needed to opt out of *clang-format* for certain code sections
Line range hint
53-64: Correct the indentation of unordered list items to maintain consistency.- * regenerate/update existing images (cover new FairSoft releases? get - latest os updates to test closer to what a user may use) - * add new images (new os releases? which ship new compiler major versions - perhaps) - * remove old images - * For macs it involves similar steps, updating homebrew, perhaps the os - or the command line tools + * regenerate/update existing images (cover new FairSoft releases? get + latest os updates to test closer to what a user may use) + * add new images (new os releases? which ship new compiler major versions + perhaps) + * remove old images + * For macs it involves similar steps, updating homebrew, perhaps the os + or the command line tools
Line range hint
56-56: Ensure headings are surrounded by blank lines for better readability and consistency.+ # Creating a new Release +
ChristianTackeGSI
force-pushed
the
docs-contributing
branch
from
9abcbff
to
504fa1b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
CONTRIBUTING.md (2)
Line range hint
24-24: Consider simplifying the phrase "at this point in time" to "currently" for conciseness.
Line range hint
53-64: Adjust the indentation of the unordered list items to match the expected levels.- * regenerate/update existing images (cover new FairSoft releases? get - latest os updates to test closer to what a user may use) - * add new images (new os releases? which ship new compiler major versions - perhaps) - * remove old images - * For macs it involves similar steps, updating homebrew, perhaps the os - or the command line tools + * regenerate/update existing images (cover new FairSoft releases? get + latest os updates to test closer to what a user may use) + * add new images (new os releases? which ship new compiler major versions + perhaps) + * remove old images + * For macs it involves similar steps, updating homebrew, perhaps the os + or the command line tools
ChristianTackeGSI
force-pushed
the
docs-contributing
branch
from
504fa1b
to
2f0a5d4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (6)
CONTRIBUTING.md (6)
Line range hint
24-24: Simplify the phrase for clarity.- These guidelines are horribly incomplete at this point in time, but one has to start somewhere ;) + These guidelines are still in development, but one has to start somewhere ;)
Line range hint
45-45: Remove redundancy in the phrase.- The set of enabled checks will evolve over time. + The set of enabled checks will evolve.
Line range hint
65-65: Correct the grammatical structure for clarity.- both in commit messages as well as in PR comments if applicable. + both in commit messages and in PR comments if applicable.
Line range hint
76-77: Correct the verb form for "opt-out".- Rarely, it is needed to opt-out of *clang-format* for certain code sections + Rarely, it is needed to opt out of *clang-format* for certain code sections
Line range hint
53-53: Standardize unordered list indentation.- * regenerate/update existing images (cover new FairSoft releases? get - latest os updates to test closer to what a user may use) - * add new images (new os releases? which ship new compiler major versions - perhaps) - * remove old images - * For macs it involves similar steps, updating homebrew, perhaps the os - or the command line tools + * regenerate/update existing images (cover new FairSoft releases? get + latest os updates to test closer to what a user may use) + * add new images (new os releases? which ship new compiler major versions + perhaps) + * remove old images + * For macs it involves similar steps, updating homebrew, perhaps the os + or the command line toolsAlso applies to: 57-57, 58-58, 61-61, 64-64, 132-132, 134-134, 136-136, 137-137
Line range hint
56-56: Ensure headings are surrounded by blank lines for better readability.+ # Creating a new Release +
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range and nitpick comments (4)
CONTRIBUTING.md (4)
Line range hint
24-24: Simplify the phrase for clarity.- These guidelines are horribly incomplete at this point in time, but one has to start somewhere ;) + These guidelines are still incomplete, but one has to start somewhere ;)
Line range hint
45-45: Remove redundancy in the phrase.- The set of enabled checks will evolve over time. + The set of enabled checks will evolve.
Line range hint
65-65: Correct the conjunction usage for grammatical accuracy.- both in commit messages as well as in PR comments if applicable. + both in commit messages and in PR comments if applicable.
Line range hint
76-77: Correct the verb form for "opt-out".- Rarely, it is needed to opt-out of *clang-format* for certain code sections + Rarely, it is needed to opt out of *clang-format* for certain code sections
ChristianTackeGSI
force-pushed
the
docs-contributing
branch
from
6eaf8b4
to
a0f4847
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (7)
CONTRIBUTING.md (7)
Line range hint
24-24: Consider simplifying "at this point in time" to "currently" to enhance clarity.- These guidelines are horribly incomplete at this point in time, but one has to start somewhere ;) + These guidelines are horribly incomplete currently, but one has to start somewhere ;)
Line range hint
45-45: The phrase "will evolve over time" can be simplified to "will evolve".- The set of enabled checks will evolve over time. + The set of enabled checks will evolve.
Line range hint
65-65: Clarify the usage of "both" by adding "and".- both in commit messages as well as in PR comments if applicable. + both in commit messages and in PR comments if applicable.
Line range hint
76-77: Correct the verb form for "opt-out".- Rarely, it is needed to opt-out of *clang-format* for certain code sections + Rarely, it is needed to opt out of *clang-format* for certain code sections
Line range hint
56-56: Ensure that headings are surrounded by blank lines for proper Markdown formatting.- # Creating a new Release + + # Creating a new Release +
Line range hint
31-31: Ensure that fenced code blocks are surrounded by blank lines for proper Markdown formatting.- ``` + + ``` +Also applies to: 81-81, 101-101
Line range hint
57-57: Ensure that lists are surrounded by blank lines for proper Markdown formatting.- * Then I compare what is currently tested via CI and decide whether it + + * Then I compare what is currently tested via CI and decide whether it +
ChristianTackeGSI
force-pushed
the
docs-contributing
branch
from
a0f4847
to
819c1bc
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
CONTRIBUTING.md (2)
133-136: Enhance the introduction clarityThe introduction could be more professional and clearer. Consider this revision:
-# Creating a new Release -(This is basically for the release manager. -So that we don't forget anything.) +# Creating a new Release + +This document outlines the step-by-step process for release managers to ensure a consistent and complete release workflow.
138-145: Structure the milestone management stepsThe milestone management section could be more actionable with numbered steps and clearer guidance.
-## Control the status - -* Take a look at the - [Milestone](https://github.com/FairRootGroup/FairRoot/milestones) - for the release - - Consider moving still open items to another milestone +## Control Release Status + +1. Review the release [milestone](https://github.com/FairRootGroup/FairRoot/milestones): + * Assess all open issues and pull requests + * Evaluate each item's criticality for the release + * Move non-critical items to future milestones
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
CONTRIBUTING.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[typographical] ~150-~150: Consider adding a comma.
Context: ...nything older, if relevant in GSI still) * Then I compare what is currently tested via ...
(IF_THEN_COMMA)
[uncategorized] ~201-~201: Possible missing comma found.
Context: ...a new release on GitHub. ## In certain cases the tag should be merged onto master: ...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint
CONTRIBUTING.md
153-153: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
155-155: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
157-157: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
158-158: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
183-183: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
191-191: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
199-199: Punctuation: '.'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
201-201: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
212-212: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
227-227: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
Be more transparent about our release process. Co-Authored: @karabowi Co-Authored: @dennisklein
Added section about complicated patch branch merging process. Added section about major version.
Our tools prefer spaces not tabs.
ChristianTackeGSI
force-pushed
the
docs-contributing
branch
from
819c1bc
to
bcab385
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
CONTRIBUTING.md (1)
133-136: Improve the introduction's clarity and professionalismConsider revising the introduction to be more formal and direct:
-# Creating a new Release -(This is basically for the release manager. -So that we don't forget anything.) +# Creating a New Release + +This document outlines the release process for release managers, ensuring all necessary steps are completed systematically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
.zenodo.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
CONTRIBUTING.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
CONTRIBUTING.md
[typographical] ~150-~150: Consider adding a comma.
Context: ...nything older, if relevant in GSI still) * Then I compare what is currently tested via ...
(IF_THEN_COMMA)
[uncategorized] ~201-~201: A comma might be missing here.
Context: ...a new release on GitHub. ## In certain cases the tag should be merged onto master: ...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🪛 Markdownlint
CONTRIBUTING.md
153-153: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
155-155: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
157-157: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
158-158: Expected: 2; Actual: 3
Unordered list indentation
(MD007, ul-indent)
183-183: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
191-191: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
199-199: Punctuation: '.'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
201-201: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
212-212: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
227-227: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
🔇 Additional comments (1)
CONTRIBUTING.md (1)
212-226: 🛠️ Refactor suggestion
Add safety checks for divergent branch merging
The current instructions for handling divergent branches need safety checks:
### Should the patch branch diverge too much from master:
+# Before proceeding:
+# 1. Create a backup branch
+git branch backup_master_$(date +%Y%m%d) master
+git branch backup_patches_$(date +%Y%m%d) v{x.y}_patches
+
+# 2. Verify the changes to be preserved
+git log --oneline master..v{x.y}_patches
+
+# 3. Proceed with the merge
git checkout master
git merge -s ours v{x.y}_patches
git checkout --detach v{x.y}_patches
git reset --soft master
git checkout master
git commit --amend -C HEAD
+
+# 4. Verify the result
+git log -p HEAD~1..HEAD
+git diff backup_patches_$(date +%Y%m%d) HEAD
+
+# 5. If everything looks correct
git push origin masterThis complex operation requires careful verification to ensure no changes are lost.
✅ Verification successful
The previous script didn't produce output because we're in a sandbox environment without git history. Let's examine the documentation to understand the context and verify if this merge workflow is documented elsewhere or if there are real examples of such patch branches.
Based on the documentation search results, I can now make a clear assessment of the review comment. The patch branch merging instructions are part of the official release process documented in CONTRIBUTING.md. The context shows this is a well-established workflow used specifically for merging patch branches back to master when creating new releases.
The review comment suggesting safety checks is verified as appropriate and valuable
The suggested safety checks are important additions because:
- The operation involves discarding changes from master, which could be dangerous without proper verification
- The backup branches provide a safety net in case something goes wrong
- The verification steps ensure the intended changes are preserved
The original review comment is correct and the suggested improvements are essential for safer release management.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if there are any existing divergent branches
git for-each-ref --format='%(refname:short)' refs/heads/v*_patches | while read branch; do
echo "Checking divergence for $branch..."
# Count commits in both directions
ahead=$(git rev-list --count master..$branch)
behind=$(git rev-list --count $branch..master)
if [ $ahead -gt 0 ] || [ $behind -gt 0 ]; then
echo "$branch is $ahead commits ahead and $behind commits behind master"
fi
done
Length of output: 84
Script:
#!/bin/bash
# Let's check if there are any examples or references to patch branches in the codebase
rg -i "patch.*branch" -A 3 -B 3
# Also check for any git workflow documentation
fd -t f "CONTRIBUTING|README" -X cat {}
Length of output: 70259
🧰 Tools
🪛 Markdownlint
212-212: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
We should be transparent on our release process.
@dennisklein , @karabowi could you please help with this PR? Either add comments or add commits to the branch.
(I'll squash things before we declare this "ready for review".)
Rendered: https://github.com/ChristianTackeGSI/FairRoot/blob/docs-contributing/CONTRIBUTING.md#user-content-creating-a-new-release
Checklist:
devbranch