[fix] Update the re pattern for the release notes section#62
[fix] Update the re pattern for the release notes section#62tdcmeehan merged 1 commit intoprestodb:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the regex used to extract release notes so that it stops before an optional trailing "Summary by Sourcery" section and adds a regression test with a new fixture covering this scenario. Class diagram for updated release note pattern in GenerateReleaseNotesTaskclassDiagram
class GenerateReleaseNotesTask {
<<task>>
- static Pattern IGNORED_COMMITS_PATTERN
# static Pattern NO_RELEASE_NOTE_PATTERN
# static Pattern RELEASE_NOTE_PATTERN
- static Pattern HEADER_PATTERN
+ void execute()
+ String extractReleaseNotes(String commitMessage)
}
class TestCheckReleaseNotesTask {
<<test>>
+ void testReleaseNotesWithTrailingSummary()
+ void testNoReleaseNotes()
+ void testValidReleaseNotes()
}
class Pattern {
<<java_util_regex>>
+ matcher(String input) Matcher
}
GenerateReleaseNotesTask --> Pattern : uses
TestCheckReleaseNotesTask --> GenerateReleaseNotesTask : tests
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 left some high level feedback:
- The updated
RELEASE_NOTE_PATTERNnow hard-requires a newline after== release note(s)? ==(\w*\n), whereas the previous pattern also handled content that continued on the same line; consider making the newline optional so existing one-line release note formats continue to match. - The new regex with the negative lookahead for
\nSummary byis quite dense; consider extracting the pattern into a named constant or adding a brief code comment explaining the intent (stop capture before theSummary by Sourceryfooter while still consuming to EOF if absent) to aid future maintainers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The updated `RELEASE_NOTE_PATTERN` now hard-requires a newline after `== release note(s)? ==` (`\w*\n`), whereas the previous pattern also handled content that continued on the same line; consider making the newline optional so existing one-line release note formats continue to match.
- The new regex with the negative lookahead for `\nSummary by` is quite dense; consider extracting the pattern into a named constant or adding a brief code comment explaining the intent (stop capture before the `Summary by Sourcery` footer while still consuming to EOF if absent) to aid future maintainers.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Since the description may contain the `Summary by Sourcery` at the end, adjust the re pattern to accommondate this situation. The negative lookahead (?!\nSummary by) only stops when it encounters "\nSummary by". If that pattern doesn't exist, it continues to the end just like the old pattern. Signed-off-by: Yihong Wang <yh.wang@ibm.com>
a039d0e to
f4600dd
Compare
addressed these two items in the changes. |
|
cc @tdcmeehan |
|
Here's another, different failure:
|
|
Also affecting |
|
@tdcmeehan, not sure what the next step is? |
|
Yes, @unidevel can help |
|
Sure, let me publish |
|
Failed, due to the maven central publising limitation, we can't publish this as executable jars. |
|
I will add a PR to publish executable jars to github, and we can then update related script to download it from github. |
|
Created PR #63 |
#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.
Since the description may contain the
Summary by Sourceryat the end, adjust the re pattern to accommondate this situation. The negative lookahead (?!\nSummary by) only stops when it encounters "\nSummary by". If that pattern doesn't exist, it continues to the end just like the old pattern.