Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(publish): Ability to merge to non-default #245

Merged
merged 9 commits into from
May 27, 2021

Conversation

BYK
Copy link
Member

@BYK BYK commented May 26, 2021

This fixes #239 and fixes #115. This fix requires us to switch
from a purely GitHub API driven publish flow to a purely local
Git driven publish flow as it is not possible to compute the
nearest merge base via any GitHub API. We rely on git log and
some parsing to be able to determine this.

The patch also effectively reverts #220 as there is now no need
for that (we are operating on the repo directly).

Test run: https://github.com/getsentry/release-tester/commits/releases/0.4.x
Also see no merges to main: https://github.com/getsentry/release-tester/commits/main

Depends on getsentry/publish#334

TODO: re-read config once we switch branches.

This fixes #239 and fixes #115. This fix requires us to switch
from a purely GitHub API driven publish flow to a purely local
Git driven publish flow as it is not possible to compute the
nearest merge base via any GitHub API. We rely on `git log` and
some parsing to be able to determine this.
@BYK BYK requested review from chadwhitacre, jan-auer and tonyo May 26, 2021 07:46
Copy link
Member

@chadwhitacre chadwhitacre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass done, bunch of questions/comments.

For next time: consider doing all the dead code removals in a separate PR.

Copy link
Member

@jan-auer jan-auer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a quick pass and looks good to me.

@@ -72,6 +72,17 @@ export const builder: CommandBuilder = (yargs: Argv) => {
'Source revision (git SHA or tag) to publish (if not release branch head)',
type: 'string',
})
.option('merge-target', {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've often seen "base branch" used as terminology for this, but "merge target" is equally clear.

@BYK BYK merged commit 41eaa00 into master May 27, 2021
@BYK BYK deleted the byk/feat/publish-merge-target branch May 27, 2021 08:56
BYK added a commit that referenced this pull request Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preparing a release not from master branch -> Publish Action Pull after merging the release branch
5 participants