Skip to content

Conversation

@janhoy
Copy link
Contributor

@janhoy janhoy commented Oct 24, 2025

https://issues.apache.org/jira/browse/SOLR-17619

Context: Running ./gradlew newChangelog generates a logchange yaml based on git branch name.

The old regex matched any number in the branch name, so SOLR-1234-New feature would add issue: 1234 to the changelog yaml, misinterpreting it as a github issue number. Reason was the extra ? after the parenthesis which makes it optional. The new regex matches strictly one of

  • PR-123
  • GH-123
  • GITHUB-123
  • #123

These will also match due to the #123 part, not the prefix

  • PR#123
  • GH#123
  • GITHUB#123

@janhoy janhoy requested a review from dsmiley October 24, 2025 22:58
@janhoy janhoy requested a review from hossman October 24, 2025 22:58
@hossman
Copy link
Member

hossman commented Oct 24, 2025

i don't know a lot about what kinds of naming conventions github users use when naming branches but i wonder if -- instead of requiring the (PR-|GH-|GITHUB-|#) prefix in the branch name to mean "subsequent digits are a github PR#" -- it might make sense to do something like this...

    def githubMatcher = gitBranch =~ /(PR-|GH-|GITHUB-|#)(\d+)/
    def githubRef = (githubMatcher && ! jiraMatcher) ? githubMatcher.group(2) : ""
    def githubLink = (githubMatcher && ! jiraMatcher) ? "issues:\n  - ${githubRef}" : ""

...ie:

  • if we've already determined the branch name has a SOLR-XXX jira component, then don't bother checking for a github PR# component (at all)
  • if it does not have a SOLR-XXX jira component, then any number in the branch name might be a github PR#

would that make more sense?
Is anyone out there naming branches SOLR-1235-fix-stuff-in-PR-678 ?

@janhoy
Copy link
Contributor Author

janhoy commented Oct 25, 2025

Since this is from branch name, we can probably skip PR detection, for two reasons:

  1. # is prob an illegal char in branch
  2. You create the branch before you know what Pr…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants