Add --remove-source-branch/--no-remove-source-branch options to pull-request:merge command
#619
Conversation
| @@ -141,6 +147,12 @@ protected function execute(InputInterface $input, OutputInterface $output) | |||
| $targetLabel = sprintf('Target: %s/%s', $targetRemote, $targetBranch); | |||
| } | |||
|
|
|||
| $authenticatedUser = $this->getParameter($input, 'authentication')['username']; | |||
| $removeSourceBranch = $input->getOption('remove-source-branch'); | |||
| if ($removeSourceBranch && $pr['user'] !== $authenticatedUser) { | |||
There was a problem hiding this comment.
This can be a problem for GitLab CE where there are no forks for pull-requests.
And you are always allowed, is it possible to add an API method to check for access?
There was a problem hiding this comment.
@sstok, I don't know if I got your point. I think this restriction isn't driven by permissions, but for respect to PR author. By instance, in my private organization I have access to delete branches from another author than me, but I think this isn't a good practice or something we should allow. IMO, the responsibility over each branch belongs only to their author, regardless your access level as merger.
phansys
force-pushed
the
remove_remote_branch_after_merge
branch
from
a46f086
to
cba9fda
Compare
phansys
force-pushed
the
remove_remote_branch_after_merge
branch
from
9ef9c42
to
546945f
Compare
| $prNumber = $input->getArgument('pr_number'); | ||
| $pr = $adapter->getPullRequest($prNumber); | ||
| $authenticatedUser = $this->getParameter($input, 'authentication')['username']; | ||
| $removeSourceBranch = $input->getOption('remove-source-branch'); |
There was a problem hiding this comment.
Maybe do a strtolower here, just for safety sake for the checks below. Or maybe the option should be changed to not take a value, so it is set to true when passed through
There was a problem hiding this comment.
Indeed, value should not be required. That's the common pattern for bool options.
There was a problem hiding this comment.
Should we add a --no-remove-source-branch option then in order to let the user express its choice to not delete the source branch without being prompted?
phansys
changed the title
remove-source-branch option to pull-request:merge command--remove-source-branch/--no-remove-source-branch options to pull-request:merge command
phansys
force-pushed
the
remove_remote_branch_after_merge
branch
from
7739ff8
to
983eaeb
Compare
phansys
force-pushed
the
remove_remote_branch_after_merge
branch
from
983eaeb
to
341830d
Compare
TODO:
pull-request:closecommandremovePullRequestSourceBranch()toAdapterinterfaceUse<- this is not possible since the command doesn't work with pull requests, it is able to delete any remote branch regardless it is part a PR or not.Adapter::removePullRequestSourceBranch()atbranch:deletecommand insteadGitHelper