feat: add support for releasing from non-master branches for legacy support #429
Conversation
…branch besides master
googlebot
added
the
cla: yes
…releases This resolves several conflicts in favor of Ben's default branch changes.
feywind
changed the title
|
|
|
I was told today that several more teams are interested in this, so I will bump it in my queue accordingly. |
…ge the actual logic.
…start updating tests
bcoe
force-pushed
the
non-master-releases
branch
from
d165148
to
7340d1f
Compare
Codecov Report
@@ Coverage Diff @@
## master #429 +/- ##
=========================================
Coverage ? 81.78%
=========================================
Files ? 43
Lines ? 5012
Branches ? 436
=========================================
Hits ? 4099
Misses ? 911
Partials ? 2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
It looks like GitHub#defaultBranch is a misnomer now and should be baseBranch which defaults to the detected default branch.
It might simplify the code if we provide the base branch as a parameter to the relative methods - the same GitHub instance could be used for multiple base (releasable) branches.
Conceptually, do we expect separate release-please invocations for each base branch -or- iterating over a list of base branches (via configuration) in the same invocation?
| @@ -312,31 +320,36 @@ export class GitHub { | |||
| maxLabels = 16, | |||
| retries = 0 | |||
| ): Promise<CommitsResponse> { | |||
| const baseBranch = await this.getDefaultBranch(); | |||
There was a problem hiding this comment.
Should this be a method argument where the caller can provide the base branch name?
There was a problem hiding this comment.
Yeah, I saw this could go either way... what we had before was that methods had to supply options.defaultBranch each time, and then it got cached, so the future invocations ignored the parameter. Which seems a bit deceptive.
I'm fine with going the other way, too, just make it have to be passed each time and caching the result value by key or something.
There was a problem hiding this comment.
Oh, I missed your last comment - my intent is that this first pass would just allow someone to run it from the command line on a onesies-basis, but I'm intending to follow it up with one that will read a list of branches from the .release-please.yaml file in a repo. So I'm good with assuming that's where things need to go, one way or another.
| @@ -226,53 +227,59 @@ export class GitHub { | |||
| maxFilesChanged = 64, | |||
| retries = 0 | |||
| ): Promise<CommitsResponse> { | |||
| const baseBranch = await this.getDefaultBranch(); | |||
There was a problem hiding this comment.
Should this be a method argument where the caller can provide the base branch name?
This PR is the ongoing work for implementing this issue:
#428