RhodeCode CE/EE

Hi Peter,

Thanks for your message. In case of the PR opened from the changelog, there's always an option to select the checkbox next to the changelog entry. (screen1)
Then rhodecode will select the ref of it. In this case (fixes-branch), but this works for bookmarks as well.

Another way of pre-selecting is adding to the url. e.g. http://rc.local:8080/pull-requests/default_reviewers-fork/pull-request/new?bookmark=fix-issue-710

We discussed your case in our team, and we think we should introduce a new option for git/hg to be able to select a default PR target, similar as we have now for landing revision. As a quick workardound, you could basically catch the url schema such as:

/repo_name/pull-request/new and redirect it to /repo_name/pull-request/new?bookmark=bookmark_name, or ?branch=some_other_branch_name

meanwhile, we don't have a better workaround.

Best,

Hi Marcin,

Thanks for your response. When you mention:

In case of the PR opened from the changelog, there's always an option to select the checkbox next to the changelog entry. (screen1) Then rhodecode will select the ref of it. In this case (fixes-branch), but this works for bookmarks as well.

This is the workflow that we're using, sorry I was not more clear. Here are the steps we've been following for opening PRs:

1. Navigate to repo's Changelog
2. Check the commit you want pulled (usually has a bookmark)
3. Click "Open new pull request for selected commit"

When following this flow, the source revision is correctly set to the bookmark but the target revision is the issue either defaulting to default or the first commit (same thing when using the URL format you mentioned). Since you mentioned this flow, were you expecting a different result?

With the version we're currently using the behavior is a bit different as the branch name that orders first is selected. This is still not perfect for us but is better as it's usually closer to where we want to be. Further, the alert dialog does not pop up when manually setting the target even if the request that fetches the list of commits to be included is still active.

Adding a new option for PR target would help. We often have several active branches so it wouldn't always be right but it would be closer than what we're seeing now.

I guess one last thing I'll ask is: Is the behavior we're seeing where commit 0 is preselected when default is closed is desirable in any circumstance? It seems like at point not selecting anything would be better than picking commit 0.

Peter-

This is the workflow that we're using, sorry I was not more clear. Here are the steps we've been following for opening PRs:

1. Navigate to repo's Changelog
2. Check the commit you want pulled (usually has a bookmark)
3. Click "Open new pull request for selected commit"

When following this flow, the source revision is correctly set to the bookmark but the target revision is the issue either defaulting to default or the first commit (same thing when using the URL format you mentioned). Since you mentioned this flow, were you expecting a different result?

Ahh yes, i confused the source/target. This is expected that default is set to 'default' branch if it exists, the problem here is that "if it exists" checks only active branches, so it then defaults to first commit.

With the version we're currently using the behavior is a bit different as the branch name that orders first is selected. This is still not perfect for us but is better as it's usually closer to where we want to be. Further, the alert dialog does not pop up when manually setting the target even if the request that fetches the list of commits to be included is still active.

Adding a new option for PR target would help. We often have several active branches so it wouldn't always be right but it would be closer than what we're seeing now.

We'd add either default branch or "empty" this way it would never load anything until you manually load the target.

I guess one last thing I'll ask is: Is the behavior we're seeing where commit 0 is preselected when default is closed is desirable in any circumstance? It seems like at point not selecting anything would be better than picking commit 0.

Yes i agree with that, selecting first commit is not a good default. We'll change that to "not selected anything".

You can change that in your code now if you want a quick patch: https://code.rhodecode.com/rhodecode-enterprise-ce/files/62aa75de625a17f6f2418b7290d707648cd74792/rhodecode/model/pull_request.py#L1402

Basically replacing
selected = 'rev:%s:%s' % (rev, rev)
with
`selected = None

and in case of also default branch, here: https://code.rhodecode.com/rhodecode-enterprise-ce/files/62aa75de625a17f6f2418b7290d707648cd74792/rhodecode/model/pull_request.py#L1396

That would do what you want for not selecting anything now.

An "empty" option would work for us.

The last last thing I'll point out is that we probably wouldn't have even noticed this if it weren't for the alert window. So, while it will be nice to prevent the server from doing unnecessary work, removing the alert would also be desirable (a user may still get this even they pick the wrong revision the quickly switch). There isn't any useful information in the alert that the user can act on. Plus, it seems like the intended behavior (cancel the existing request and make a new one) and thus shouldn't result in an error to the user.

Any idea on the release/time frame that such a feature would be added in?

Thanks again for looking into this,
Peter

Hi Peter,

Yes the alert for canceled request is bad. We'll fix that together with the pick revision in case no default branch exists. Those two things for sure will go in 4.11 which we plan in next ~2 weeks.

As for the picker i think 4.12, so next 4-6 weeks after 4.11

Best,

Commit 829818ee6228 by Marcin Kuzminski marcin@rhodecode.com on default branch references this issue. https://internal-code.rhodecode.com/rhodecode-enterprise-ce/changeset/829818ee62286d2c969d607505b7d3a5d368d3c5

Not sure where you are with this, but I wanted to mention this: some of my colleagues have created bash aliases to help them open pull requests, the one missing ingredient is the ability to set the target branch via the URL string. Here's what they're doing:

alias rp='xdg-open "$(hg paths default)/pull-request/new?bookmark=$(hg parent -T {bookmarks})"'

I can open a new request for that additional parameter if you'd like.

Hi Kieran,

THis sounds like a good idea, would be great if you can open a PR, we love contributions! :)