Add support for default branch detection #254
Conversation
| @@ -51,6 +51,8 @@ def __init__(self, pr_remote, commit_sha1, branches, | |||
| if dry_run: | |||
| click.echo("Dry run requested, listing expected command sequence") | |||
|
|
|||
| self.remember_current_branch() | |||
There was a problem hiding this comment.
The method is called only once, let's inline it.
There was a problem hiding this comment.
I think named things are better readable within the code flow, don't you?
There was a problem hiding this comment.
Well, I don't care too much.
My the only objection is the call hides _start_branch initialization by moving attribute assignment from constructor to method.
Also, cherry-picker doesn't use underscores for attribute names.
Let's rename self._start_branch for consistency.
There was a problem hiding this comment.
Yeah, I've also had this concern and then saw this pattern being used all over the place. I think, I can simplify this.
| @@ -104,6 +106,7 @@ def run_cmd(self, cmd): | |||
| return | |||
| output = subprocess.check_output(cmd, stderr=subprocess.STDOUT) | |||
| click.echo(output.decode('utf-8')) | |||
| return output | |||
There was a problem hiding this comment.
There the function's result is used?
I don't see any application.
There was a problem hiding this comment.
Yeah, it was used in the previous commit. Now it's not.
Based on suggestions of @serhiy-storchaka @terryjreedy in python#250: Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu> Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
webknjaz
force-pushed
the
feature/autodetect-branch-name
branch
2 times, most recently
from
f09b0fd
to
5a7efe8
Compare
|
Fair enough! I've changed it as suggested and added doc to readme. |
And fix test name shaddowing
Fix #250