GH-51098: [Release] Update CHANGELOG.md as a post release task - #51117
GH-51098: [Release] Update CHANGELOG.md as a post release task#51117kou wants to merge 3 commits into
Conversation
|
|
| if f'# Apache Arrow {version} (' in current_content: | ||
| raise ValueError( | ||
| f'CHANGELOG.md already contains the changelog of {version}!') |
| if [ ${BUMP_CHANGELOG} -gt 0 ]; then | ||
| echo "Updating CHANGELOG.md for ${version}" | ||
| archery release changelog add ${version} | ||
| git add "${SOURCE_DIR}/../../CHANGELOG.md" | ||
| git commit -m "MINOR: [Release] Update CHANGELOG.md for ${version}" | ||
| fi |
There was a problem hiding this comment.
I don't feel too strongly about needing to rename the file.
There was a problem hiding this comment.
Copied existing entries from CHANGELOG.md in existing tags.
There was a problem hiding this comment.
Pull request overview
This PR updates Arrow’s release automation so CHANGELOG.md on the default branch stays current by generating and committing changelog entries as part of the post-release version bump workflow, and adds supporting test/archery changes to make that workflow testable and idempotent.
Changes:
- Add a post-release step in
post-10-bump-versions.shto runarchery release changelog addand commitCHANGELOG.md. - Add/extend Ruby release-script tests to validate the changelog update behavior with deterministic output.
- Add an environment-controlled limit on processed GitHub issues in Archery release tooling to speed up tests, and adjust
changelog addto prevent duplicate insertion by checking file content.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| dev/release/post-10-bump-versions.sh | Adds an optional post-release changelog update + commit step. |
| dev/release/post-10-bump-versions-test.rb | Adds a new changelog test and time normalization helper for stable assertions. |
| dev/archery/archery/release/core.py | Adds a test-oriented env var to limit fetched issues for changelog generation. |
| dev/archery/archery/release/cli.py | Makes changelog add reject duplicates based on existing CHANGELOG.md content. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| max_issues = os.environ.get("ARCHERY_MAX_PROJECT_ISSUES") | ||
| if max_issues is not None: | ||
| issues = issues[:int(max_issues)] | ||
| return list(map(Issue.from_github, issues)) |
| def normalize_time(string) | ||
| string.gsub(/\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}[+-]\d{2}:\d{2}/) do | ||
| normalized_time | ||
| end | ||
| end |
4221612 to
ff70bc5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
dev/archery/archery/release/core.py:143
- ARCHERY_MAX_PROJECT_ISSUES is parsed with int(max_issues) without validation. If the env var is set to a non-integer (or a negative value), this will raise an unhelpful exception and break release tooling unexpectedly. Validate and raise a clearer error (or ignore the env var) before slicing.
# This is only for testing. We can limit the number of issues
# to be processed for faster testing.
max_issues = os.environ.get("ARCHERY_MAX_PROJECT_ISSUES")
if max_issues is not None:
issues = issues[:int(max_issues)]
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
dev/archery/archery/release/core.py:143
- ARCHERY_MAX_PROJECT_ISSUES is parsed with int() without validation. If the environment variable is set to a non-integer (or a negative value), this will raise (or behave unexpectedly) in normal CLI usage, even though the knob is intended only for tests. Consider validating/handling parse errors and negative values to avoid surprising crashes.
# This is only for testing. We can limit the number of issues
# to be processed for faster testing.
max_issues = os.environ.get("ARCHERY_MAX_PROJECT_ISSUES")
if max_issues is not None:
issues = issues[:int(max_issues)]
There was a problem hiding this comment.
🔵 Needs a closer look
A few changes introduce avoidable operational brittleness (credentials handling) and unvalidated env parsing that should be tightened before relying on this in the release workflow.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
dev/release/post-10-bump-versions.sh:79
archery release ... changelog addrelies on GitHub API access, but this script doesn't ensureGH_TOKENis available (unlike other release scripts that sourceutils-env.sh). This can make the post-release step flaky due to unauthenticated rate limits or missing credentials.
dev/release/post-10-bump-versions-test.rb:24@envis derived from the current working directory viaFile.expand_path(...), which makes the test sensitive to where it is invoked from. You already computetop_dirfrom__dir__; derive the.envpath from that for stability.
dev/archery/archery/release/core.py:143
ARCHERY_MAX_PROJECT_ISSUESis converted withint(...)without validation; a non-integer (or negative) value will raise an unhelpful exception from deep inside the release tooling. Since this is an env knob, it’s worth validating and failing with a clear message.
# This is only for testing. We can limit the number of issues
# to be processed for faster testing.
max_issues = os.environ.get("ARCHERY_MAX_PROJECT_ISSUES")
if max_issues is not None:
issues = issues[:int(max_issues)]
- Files reviewed: 6/7 changed files
- Comments generated: 0 new
- Review effort level: Lite
| include ../../NOTICE.txt | ||
|
|
||
| include archery/reports/* | ||
| include archery/templates/* |
There was a problem hiding this comment.
3830f05 introduced MANIFEST.in but it used templates/ not reports/.
Rationale for this change
In the current release workflow, we update
CHANGELOG.mdonly in the maintenance branches. SoCHANGELOG.mdin main misses entries.We can update
CHANGELOG.mdin our release workflow.What changes are included in this PR?
archery release changelog addinpost-10-bump-versions.shCHANGELOG.mdAre these changes tested?
Yes.
Are there any user-facing changes?
Yes.