Update create-release script to account for squash merge commits - #48154
Conversation
| re = /Merge pull request #(?<pr>\d*) from code-dot-org\/(?<branch>\S*) (?<description>.*)/ | ||
| # Finds PRs that were either merged or squash-merged | ||
| merge_commit_pattern = /Merge pull request #(?<pr>\d*) from code-dot-org\/(?<branch>\S*) (?<description>.*)/ | ||
| squash_merge_pattern = /(?<description>.*) \(#(?<pr>\d*)\)/ |
There was a problem hiding this comment.
can you check if you get any false positives with this regex? it looks a bit more permissive than the one I experimented with earlier. improvements could include looking for at least 5 digits, and requiring a newline after the ).
There was a problem hiding this comment.
Good question - looking for at least 5 digits sounds like a good improvement. I noticed a couple other false positives from this from some older commits, so I'll tinker with this syntax a bit more.
There was a problem hiding this comment.
any luck Sanchit? I didn't see any false positives in the past 5 years with 5 digit minimum and trailing newline as part of the regex.
There was a problem hiding this comment.
@davidsbailey thanks for the ping - haven't had a chance to test this in a while, but I think the only false positives I saw were from >5 years ago, so seems pretty safe to go with what you have. I'll confirm and update this.
davidsbailey
left a comment
There was a problem hiding this comment.
thanks for following up!
Updates the
create-releasescript to include squash merge commits when creating a new release changelog. The method of identifying squash merges here is looking for(#\d+)e.g.(#12345)which isn't necessarily super robust, but I thought it best to at least get this added for now. Part of the challenge is that squash merge commits are just commits, so they'd need to be identified based on commit message. However, if we're good about using the default commit message when squash merging, then this should pick up the changes correctly.Links
Slack discussion: https://codedotorg.slack.com/archives/C0T0PNTM3/p1662668103782079?thread_ts=1662572922.185429&cid=C0T0PNTM3
Testing story
Tested this out with the last 5 DTPs (generating output only). Results below:
DTP: #48146
Expected: 8 changes (2 squash merge commits / 6 merges)
Generated output:
Changelog:
DTP: #48124
Expected: 16 changes (6 squash merge commits / 10 merges)
Generated output:
Changelog:
DTP: #48094
Expected: 9 changes (3 squash merge commits / 6 merges)
Generated output:
Changelog:
DTP: #48074
Expected: 10 changes (4 squash merge commits / 6 merges)
Generated output:
Changelog:
DTP: #48059
Expected: 9 changes (2 squash merge commits / 7 merges
Generated output:
Changelog:
Rails/BlankDeployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: