Repository navigation
minor: release process - #18681
minor: release process#18681cecemei wants to merge 14 commits into
Conversation
|
This pull request has been marked as stale due to 60 days of inactivity. |
|
This pull request/issue has been closed due to lack of activity. If you think that |
|
This pull request has been marked as stale due to 60 days of inactivity. |
|
This pull request/issue has been closed due to lack of activity. If you think that |
| print("Tagging Pull Request {} with milestone {}".format(pr_number, milestone)) | ||
| url = "https://api.github.com/repos/apache/druid/issues/{}".format(pr_number) | ||
| requests.patch(url, json=milestone_json, auth=(github_username, os.environ["GIT_TOKEN"])) | ||
| if os.environ.get("DRY_RUN", "true").lower() == "false": |
There was a problem hiding this comment.
Please call out the DRY_RUN env variable in the help text of this script and maybe in the comments too.
|
|
||
| ```bash | ||
| $ git checkout origin/master | ||
| $ git checkout origin/37.0.0 |
There was a problem hiding this comment.
Please add a line above mentioning that this is for hotfix/minor version releases.
FrankChen021
left a comment
There was a problem hiding this comment.
🟡 Changes recommended
Reviewed all 4 of 4 changed files in the supplied merge-base diff. The release-guide changes were checked against the three modified helper scripts and the surrounding milestone/backport workflow; the scripts have no repository test coverage, so the assessment is static.
Validation: git diff --check was run on the four changed files and reported trailing whitespace on added documentation lines. No builds, installs, formatters, or broad tests were run, per the requested static-review scope.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 1 |
| P3 | 1 |
| Total | 3 |
This is an automated review by Codex GPT-5.6-Luna(max)
After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.
| print("Tagging Pull Request {} with milestone {}".format(pr_number, milestone)) | ||
| url = "https://api.github.com/repos/apache/druid/issues/{}".format(pr_number) | ||
| requests.patch(url, json=milestone_json, auth=(github_username, os.environ["GIT_TOKEN"])) | ||
| if os.environ.get("DRY_RUN", "true").lower() == "false": |
There was a problem hiding this comment.
[P1] Default dry-run disables milestone tagging
Finding: A normal invocation now takes the DRY_RUN default of true, so every missing milestone is only reported and the GitHub PATCH is skipped. The release guide still presents this script as the tool that tags missing PRs, and find-missing-backports.py relies on those milestones; following the documented workflow therefore leaves PRs untagged and can make the later backport check silently incomplete.
Suggestion: Keep the normal invocation mutating (or add an explicit --dry-run opt-in) and document the safe preview mode separately.
|
|
||
| ```bash | ||
| $ svn add 0.17.0-rc3 | ||
| $ cp ../druid-release/distribution/target/apache* ./37.0.0-rc1/. |
There was a problem hiding this comment.
[P2] Artifact copy targets a directory that was never created
Finding: Immediately after checking out dist/dev/druid, the documented command copies into ./37.0.0-rc1/., but no step creates that directory. From a clean checkout, cp fails with a missing-destination error and the release artifacts are never staged for svn add.
Suggestion: Create the release-candidate directory before copying, or use a checkout/copy command that creates the destination explicitly.
| ### Cancelling a vote | ||
|
|
||
| If for any reason during the Druid PMC vote a blocking issue becomes apparent, a vote should be officially cancelled by sending an email with the following subject line: `[CANCEL][VOTE] Release Apache Druid 0.17.0 [RC3]` and the reasons for the cancellation in the body. | ||
| If for any reason during the Druid PMC vote a blocking issue becomes apparent, a vote should be officially cancelled by sending an email with the following subject line: `[CANCEL][VOTE] Release Apache Druid 37.0.0 [RC3]` and the reasons for the cancellation in the body. |
There was a problem hiding this comment.
[P3] Cancellation template names the wrong release candidate
Finding: The voting and final-release instructions in this same guide use druid-37.0.0-rc2, but the cancellation subject was changed to 37.0.0 [RC3]. Copying the cancellation template for the documented vote would identify a different, likely nonexistent candidate and make the release-thread record inaccurate.
Suggestion: Use the same candidate placeholder/version as the vote and final-release examples, or make the RC number explicitly variable throughout the template.
FrankChen021
left a comment
There was a problem hiding this comment.
🟢 Approval recommended
Rechecked the three findings from the previous review against this head: the milestone script now applies tags by default and documents APPLY_TAGS=false preview mode, the release candidate directory is created before copying, and the cancellation subject matches the RC2 vote and final-release examples. One non-blocking issue remains: most links in the new checklist do not navigate because their fragments use uppercase letters while GitHub heading IDs are lowercase. This affects navigation only; the release instructions remain readable and can be corrected after merging.
Coverage: reviewed all 4 of 4 changed files: distribution/asf-release-process-guide.md, distribution/bin/find-missing-backports.py, distribution/bin/get-milestone-prs.py, and distribution/bin/tag-missing-milestones.py. Started with the incremental diff, then inspected the full diff and surrounding release workflow and scripts; rechecked all three prior findings.
Validation: git diff --check 443688fea12b1fee730f38f3aef87462573b475e...HEAD reported trailing whitespace on 10 added guide lines. The prepared worktree is clean. Static review only; no builds or tests were run.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 0 |
| P3 | 1 |
| Total | 1 |
This is an automated review by Codex GPT-5.6-Luna(max)
After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.
| ## 🚀 Release Checklist | ||
|
|
||
| - [ ] [Prior to starting, initial setup for release access](#prior-to-starting-initial-setup-for-release-access) | ||
| - [ ] [SVN access](#SVN-access) |
There was a problem hiding this comment.
[P3] Lowercase the checklist section fragments
Finding: The new checklist uses fragments such as #SVN-access, but GitHub generates lowercase heading IDs such as #svn-access. Most checklist links therefore do not jump to their sections, making the new navigation unreliable.
Suggestion: Lowercase each fragment so it matches the generated ID for its target heading.
Release process.
This PR has: