Add auto-merge manifest option #34
Conversation
dgellow
requested review from
rattrayalex,
RobertCraigie and
marcel-stainless
|
I forgot to add a check for minor/patch, from what I understood auto-merge should only be for non-breaking changes. |
|
I believe my primary comments have been resolved, I think this is good to ship! |
There was a problem hiding this comment.
Sorry for the churn; let's do the more granular commit message filtering in this PR so we don't generate too many spam releases based on internal/minor changes, per Robert's comment here.
dgellow
force-pushed
the
sam/sta-2296-auto-merge-release-prs-when-possible
branch
from
21f5ee7 to
53b0d1b
Compare
| autoMerge: { | ||
| mergeMethod: 'rebase', | ||
| versionBumpFilter: ['minor'], | ||
| conventionalCommitFilter: [{type: 'fix', scope: 'api'}], | ||
| }, |
test/manifest.ts
Outdated
| sinon.assert.calledWith( | ||
| createPullRequestStub, | ||
| sinon.match.has( | ||
| 'headBranchName', | ||
| 'release-please/branches/main/components/a' | ||
| ), | ||
| 'main', | ||
| 'main', | ||
| sinon.match.string, | ||
| sinon.match.array, | ||
| sinon.match({ | ||
| autoMerge: undefined, | ||
| }) | ||
| ); | ||
| sinon.assert.calledWith( | ||
| createPullRequestStub, | ||
| sinon.match.has( | ||
| 'headBranchName', | ||
| 'release-please/branches/main/components/b' | ||
| ), | ||
| 'main', | ||
| 'main', | ||
| sinon.match.string, | ||
| sinon.match.array, | ||
| sinon.match({ | ||
| autoMerge: {mergeMethod: 'rebase'}, | ||
| }) | ||
| ); | ||
| sinon.assert.calledWith( | ||
| createPullRequestStub, | ||
| sinon.match.has( | ||
| 'headBranchName', | ||
| 'release-please/branches/main/components/c' | ||
| ), | ||
| 'main', | ||
| 'main', | ||
| sinon.match.string, | ||
| sinon.match.array, | ||
| sinon.match({ | ||
| autoMerge: undefined, | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Asserts for the autoMerge parameter when github.createPullRequest() is called.
| autoMerge: { | ||
| mergeMethod: 'rebase', | ||
| conventionalCommitFilter: [ | ||
| {type: 'fix'}, | ||
| {type: 'feat', scope: 'api'}, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
@rattrayalex That's the config I expect Stainless to be using.
There was a problem hiding this comment.
I haven't done careful code review of the contents of the recent commits, but the core stuff sounds good to me and I think the meat of the code was previously reviewed!
@marcel-stainless may want to give it another look over, I'm not sure, but probably okay to fast-follow with fixes that come out of that.
dgellow
deleted the
sam/sta-2296-auto-merge-release-prs-when-possible
branch
This pull request adds an
autoMergemanifest option that can be passed to the factoryfromManifest(). When set, release-please attempts to enable auto-merge for release pull requests.Note that auto-merge can only be enabled on a pull request with branch protections (such as required reviewers, required CI checks, etc).