[fix] Add release notes summary to a file when it is too big#65
[fix] Add release notes summary to a file when it is too big#65tdcmeehan merged 1 commit intoprestodb:masterfrom
Conversation
Reviewer's GuideImplements logic to handle GitHub PR body length limits for generated release notes by optionally writing the full summary to a versioned file and using a truncated body in the PR, and updates the release workflow to support mock publishing without failing the job. Sequence diagram for generating release notes PR with summary file fallbacksequenceDiagram
participant ReleaseService as PrestoReleaseService
participant Task as GenerateReleaseNotesTask
participant Git as GitClient
participant FS as FileSystem
participant GH as GithubGraphQlAction
ReleaseService->>Task: run()
Task->>Git: createBranch(release-notes-{version})
Task->>Task: build releaseNotes
Task->>Task: build releaseNotesSummary
Task->>Task: fullSummary = releaseNotesSummary + RELEASE_NOTES_FOOTER
alt fullSummary length > githubPrBodyLimit
Task->>Task: build truncationMessage
Task->>Task: compute availableSpace
Task->>Task: truncatedSummary = releaseNotesSummary.substring(...)
Task->>Task: prBody = truncatedSummary + RELEASE_NOTES_FOOTER + truncationMessage
Note over Task,FS: summaryFileCreated = true
else fullSummary within limit
Task->>Task: prBody = fullSummary
Note over Task,FS: summaryFileCreated = false
end
Task->>Git: checkout(branch release-notes-{version})
Task->>FS: update RELEASE_NOTES_LIST_FILE
alt summaryFileCreated is true
Task->>FS: write release-notes-missing-{version}.md with fullSummary
end
Task->>Git: commit release notes changes
Task->>Git: push origin release-notes-{version}
Task->>GH: createPullRequest(base=master, head=origin:release-notes-{version}, title=docs: Add release notes for {version}, body=prBody)
GH-->>Task: releaseNotesPullRequest URL
Task-->>ReleaseService: done
Class diagram for updated GenerateReleaseNotesTaskclassDiagram
class GenerateReleaseNotesTask {
- String releaseNotesSummary
- String RELEASE_NOTES_FOOTER
- GitClient git
- Repository repository
- String RELEASE_NOTES_LIST_FILE
+ void run()
- void createReleaseNotesCommit(String version, String branch, String releaseNotes, Optional~String~ fullSummary)
}
class GitClient {
+ void checkout(Optional~String~ remote, Optional~String~ branch)
+ void push(String remote, String branch, boolean force)
}
class Repository {
+ String getOriginName()
}
class Optional~String~ {
+ boolean isPresent()
+ String get()
+ static Optional~String~ of(String value)
+ static Optional~String~ empty()
}
GenerateReleaseNotesTask --> GitClient : uses
GenerateReleaseNotesTask --> Repository : uses
GenerateReleaseNotesTask --> Optional~String~ : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
availableSpacecalculation for the truncated summary can go negative if the truncation message + footer + buffer exceedgithubPrBodyLimit, which would causesubstringto throw; consider wrapping it inMath.max(0, ...)before using it. - The relative link in
truncationMessage(../blob/release-notes-%s/release-notes-missing-%s.md) may not resolve correctly from the PR body; it would be more robust to use an absolute GitHub URL built from the origin repo/branch or a simple filename reference without a relative path.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `availableSpace` calculation for the truncated summary can go negative if the truncation message + footer + buffer exceed `githubPrBodyLimit`, which would cause `substring` to throw; consider wrapping it in `Math.max(0, ...)` before using it.
- The relative link in `truncationMessage` (`../blob/release-notes-%s/release-notes-missing-%s.md`) may not resolve correctly from the PR body; it would be more robust to use an absolute GitHub URL built from the origin repo/branch or a simple filename reference without a relative path.
## Individual Comments
### Comment 1
<location path="presto-release-tools/src/main/java/com/facebook/presto/release/tasks/GenerateReleaseNotesTask.java" line_range="204-213" />
<code_context>
+ if (fullSummary.length() > githubPrBodyLimit) {
</code_context>
<issue_to_address>
**issue:** Guard against negative `availableSpace` when truncating the summary
If `truncationMessage.length() + RELEASE_NOTES_FOOTER.length() + 100` is greater than `githubPrBodyLimit`, `availableSpace` becomes negative, causing `substring(0, availableSpace)` to throw. Please clamp `availableSpace` at 0 (e.g., `Math.max(0, availableSpace)`) or short‑circuit to a minimal fallback body when `availableSpace <= 0`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
...-release-tools/src/main/java/com/facebook/presto/release/tasks/GenerateReleaseNotesTask.java
Show resolved
Hide resolved
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The Markdown link in the truncation notice (
../blob/release-notes-%s/release-notes-missing-%s.md) is unlikely to resolve correctly from a PR description; consider using a repo-relative path (e.g./blob/release-notes-%s/...) or just referencing the filename without a link so users can reliably locate the file. - When truncating
releaseNotesSummaryto fit the PR body limit, the substring is cut at an arbitrary character boundary; consider trimming at the last newline or word boundary withinavailableSpaceto avoid broken Markdown and mid-word truncation in the rendered PR description.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Markdown link in the truncation notice (`../blob/release-notes-%s/release-notes-missing-%s.md`) is unlikely to resolve correctly from a PR description; consider using a repo-relative path (e.g. `/blob/release-notes-%s/...`) or just referencing the filename without a link so users can reliably locate the file.
- When truncating `releaseNotesSummary` to fit the PR body limit, the substring is cut at an arbitrary character boundary; consider trimming at the last newline or word boundary within `availableSpace` to avoid broken Markdown and mid-word truncation in the rendered PR description.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
steveburnett
left a comment
There was a problem hiding this comment.
LGTM!
Makes sense.
Note for the future, if relevant:
The summary states that the release owner must remove the release-notes-missing-<version>.md file before the release notes PR is merged. If we plan to keep this workaround for the future, suggest automating the file deletion as part of the release process.
#27466) ## Description 1. presto-release-tools can not be fetched due to maven central publishing limitation 2. check maven central publishing requirements 3. add required `<name>` field to presto-lance ## Motivation and Context Depends on PRs: - prestodb/presto-release-tools#65 - prestodb/presto-release-tools#64 - prestodb/presto-release-tools#63 - prestodb/presto-release-tools#62 ## Impact CI ## Test Plan Tested with: 1. release note check action: https://github.com/prestodb/presto/actions/runs/23815800338/job/69414865062?pr=27466 2. maven central publishing requirements check: https://github.com/unix280/presto/actions/runs/23812452401/job/69402863222#step:8:76 3. Prepare release action in presto => https://github.com/unix280/presto/actions/runs/23812844511 4. Release notes PR => unix280#52 5. Release notes missing list file => https://github.com/unix280/presto/blob/release-notes-0.297/release-notes-missing-0.297.md ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. - [ ] If adding new dependencies, verified they have an [OpenSSF Scorecard](https://securityscorecards.dev/#the-checks) score of 5.0 or higher (or obtained explicit TSC approval for lower scores). ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == NO RELEASE NOTE == ``` ## Summary by Sourcery Update CI and release scripts to retrieve presto-release-tools from GitHub releases using a configurable version instead of Maven Central. CI: - Change release-notes-check workflow to download the presto-release-tools executable directly from GitHub releases and run it from a temporary path. - Make the release-notes-check workflow use a configurable RELEASE_TOOLS_VERSION with a default of 0.13. Deployment: - Update release preparation workflow and release-notes script to use a configurable RELEASE_TOOLS_VERSION (default 0.13) and fetch presto-release-tools from GitHub releases instead of Maven. Chores: - Align release tooling version and retrieval method across CI workflows and release scripts.
prestodb#27466) ## Description 1. presto-release-tools can not be fetched due to maven central publishing limitation 2. check maven central publishing requirements 3. add required `<name>` field to presto-lance ## Motivation and Context Depends on PRs: - prestodb/presto-release-tools#65 - prestodb/presto-release-tools#64 - prestodb/presto-release-tools#63 - prestodb/presto-release-tools#62 ## Impact CI ## Test Plan Tested with: 1. release note check action: https://github.com/prestodb/presto/actions/runs/23815800338/job/69414865062?pr=27466 2. maven central publishing requirements check: https://github.com/unix280/presto/actions/runs/23812452401/job/69402863222#step:8:76 3. Prepare release action in presto => https://github.com/unix280/presto/actions/runs/23812844511 4. Release notes PR => unix280#52 5. Release notes missing list file => https://github.com/unix280/presto/blob/release-notes-0.297/release-notes-missing-0.297.md ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. - [ ] If adding new dependencies, verified they have an [OpenSSF Scorecard](https://securityscorecards.dev/#the-checks) score of 5.0 or higher (or obtained explicit TSC approval for lower scores). ## Release Notes Please follow [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines) and fill in the release notes below. ``` == NO RELEASE NOTE == ``` ## Summary by Sourcery Update CI and release scripts to retrieve presto-release-tools from GitHub releases using a configurable version instead of Maven Central. CI: - Change release-notes-check workflow to download the presto-release-tools executable directly from GitHub releases and run it from a temporary path. - Make the release-notes-check workflow use a configurable RELEASE_TOOLS_VERSION with a default of 0.13. Deployment: - Update release preparation workflow and release-notes script to use a configurable RELEASE_TOOLS_VERSION (default 0.13) and fetch presto-release-tools from GitHub releases instead of Maven. Chores: - Align release tooling version and retrieval method across CI workflows and release scripts.
This PR fixed the error when geneating release notes by adding the summary to a file with name
release-notes-missing-<version>.mdunder presto root folder, release owner need to remove this file before the release notes PR is meges.Tested with: