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

ref: Use default branch as merge target instead of parent detection #355

Merged
merged 1 commit into from
Mar 1, 2022

Conversation

kamilogorek
Copy link
Contributor

This is WiP to fix #327

We remove automatic branch detection, use the default as the main source of truth, but allow for overriding it through the GH Action input (this needs to be added per repository, inside .github/workflows/release.yml in with clause).

Related required issues:
getsentry/action-prepare-release#23
getsentry/publish#838

@kamilogorek
Copy link
Contributor Author

cc @BYK

@BYK
Copy link
Member

BYK commented Feb 17, 2022

This will require reopening #239. I think we can still consider using git merge-target and then find the branch name from there?

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.

Thanks, LGTM.

Could you please add this to the readme as well? The merge target is not listed at all in the publish section yet.

@jan-auer
Copy link
Member

jan-auer commented Mar 1, 2022

@BYK can you elaborate why this would reopen #239? Together with the linked PRs, this gives enough control to choose the merge target manually in the few cases this is necessary.

@kamilogorek kamilogorek merged commit 44a7d40 into master Mar 1, 2022
@kamilogorek kamilogorek deleted the merge-branch branch March 1, 2022 15:42
@lobsterkatie
Copy link
Member

this needs to be added per repository, inside .github/workflows/release.yml in with clause

Specifically what needs to be added? Could you link to an example here for individual repo maintainers to follow?

@kamilogorek
Copy link
Contributor Author

@lobsterkatie I updated the readme yesterday here - https://github.com/getsentry/publish#merge-target

@BYK
Copy link
Member

BYK commented Mar 3, 2022

@jan-auer

@BYK can you elaborate why this would reopen #239? Together with the linked PRs, this gives enough control to choose the merge target manually in the few cases this is necessary.

Oh, right my bad. I thought this PR was doing away with the merge-target option completely. It is now more error-prone and cumbersome to achieve #239 as one has to remember to both have this config in their release workflow and not forget to add the correct merge target manually without any validation ahead of time.

I agree that it is still better than merging to the wrong target for the most common case but the experience and safety is far from being ideal.

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.

Last step of publish is selecting the wrong merge target
4 participants