Skip to content

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented Apr 1, 2016

As we are already using the API to retrieve the pull request title, also retrieve the base branch.

This makes sure that pull requests for 0.12 automatically end up in 0.12, and pull requests for master automatically end up in master, and so on.

It prints the branch it is about to merge into. It is still possible to override the branch from the command line or using the githubmerge.branch git option.

@sipa
Copy link
Member

sipa commented Apr 1, 2016

Concept ACK; this has been a feature I've wanted for a while.

@jonasschnelli
Copy link
Contributor

Nice! Concept ACK.
Great safety measure also.

@maflcko
Copy link
Member

maflcko commented Apr 1, 2016

utACK c1cff21

Copy link
Member

Choose a reason for hiding this comment

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

Nit: Couldn't this be just one line?

Something like

branch = args.branch or opt_branch or info['base']['ref'] or 'master'

Copy link
Member Author

Choose a reason for hiding this comment

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

That's much shorter, also seems to work:

>>> None or 'a' or 'b' or 'c'
'a'
>>> None or None or 'b' or 'c'
'b'
>>> None or None or None or 'c'
'c'
>>> 'f' or None or None or 'c'
'f'

I think the subtle difference in this case is that '' is seen as False (hence the use of is None. But doesn't matter here, an empty branch name is pointless.

As we are already using the API to retrieve the pull request
title, also retrieve the base branch.

This makes sure that pull requests for 0.12 automatically end up in
0.12, and pull requests for master automatically end up in master,
and so on.

It is still possible to override the branch from the command line
or using the `githubmerge.branch` git option.
@laanwj laanwj force-pushed the 2016_04_github_merge_autobranch branch from c1cff21 to 10d3ae1 Compare April 2, 2016 06:13
@btcdrak
Copy link
Contributor

btcdrak commented Apr 5, 2016

utACK 10d3ae1

@laanwj
Copy link
Member Author

laanwj commented Apr 5, 2016

I've been using this version for a few days, both on master and 0.12, seems to work.
Tested ACK 10d3ae1

@laanwj laanwj merged commit 10d3ae1 into bitcoin:master Apr 5, 2016
laanwj added a commit that referenced this pull request Apr 5, 2016
10d3ae1 devtools: Auto-set branch to merge to in github-merge (Wladimir J. van der Laan)
codablock pushed a commit to codablock/dash that referenced this pull request Dec 19, 2017
…erge

10d3ae1 devtools: Auto-set branch to merge to in github-merge (Wladimir J. van der Laan)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants