New MR default target branch is default of target.
Created by: cirosantilli
Fixes ACCEPTING MR at http://feedback.gitlab.com/forums/176466-general/suggestions/5518180-smarter-merge-request-target-repo-and-branch-form-
One liner changer + tests ;~
Behavior before this PR: when creating a new MR, the target branch is the first one alphabetically.
Behavior after this PR: when creating a new MR, the target branch is the default of the target repo (which is currently always the current repo).
help me
Works well on interactive tests, but for the unit tests I am unable to change the current branch.
I have tried things like
@project.change_head(branch)
and
click_link("Settings")
select2(branch, from: "Default Branch")
click_button("Save changes")
and many variations without success.
Once this is settled I'll remove the test comments and prints.
Merge request reports
Activity
Created by: cirosantilli
@kemenaran If you like this, you might also want this to help with MRs to forked repos.
By Administrator on 2014-02-24T13:37:30 (imported from GitLab project)
By Administrator on 2014-02-24T13:37:30 (imported from GitLab)
Created by: dblessing
Hi @cirosantilli. Thanks for your contribution. Let me know if I'm misunderstanding this, but I think https://github.com/gitlabhq/gitlabhq/commit/1ff1b6d816cd1150df436a8ff56502e8adfa1b0e may solve the issue you're describing. That commit appears to make it so the target project is the upstream (forked from) project and the branch is the default branch, if not specified. I tested this on the latest development version and it seems to work. What do you think?
By Administrator on 2014-03-27T00:26:28 (imported from GitLab project)
By Administrator on 2014-03-27T00:26:28 (imported from GitLab)
Created by: dblessing
I looked at the referenced commit again. I think it all but solves the issue you're describing. I think just one line from your pull request here would finish the deal.
If you look at https://github.com/gitlabhq/gitlabhq/blob/1ff1b6d816cd1150df436a8ff56502e8adfa1b0e/app/controllers/projects/merge_requests_controller.rb#L66 you can see that there is a "target_branch" value on the merge_request object. The part that wasn't implemented was the form selection. If we add https://github.com/cirosantilli/booktree/blob/mr-default-dest-branch/app/views/projects/merge_requests/_form.html.haml#L32 from your pull request I think this would work.
Let me know what you think. If you think this will work, would you like to amend your pull request? Thanks.
By Administrator on 2014-03-27T00:32:46 (imported from GitLab project)
By Administrator on 2014-03-27T00:32:46 (imported from GitLab)
Created by: cirosantilli
@dblessing thanks for the link! On master and gitlab.com, everything #5719 proposed to do has been done, so it is probably that PR that implemented it:
- default target repo is original fork (was not done here)
- default target branch is default branch of target repo (exactly what this merge request intended to do)
If you reproduce, we can close this, as it is a subset of something already implemented.
Work duplication due to multiple communication channels =(
By Administrator on 2014-03-27T14:09:00 (imported from GitLab project)
By Administrator on 2014-03-27T14:09:00 (imported from GitLab)
Created by: dblessing
I can confirm that the behavior described here is now present in GitLab. Sorry for the confusion, @cirosantilli. Thanks for your willingness to contribute. It's definitely appreciated.
This PR can be closed. @razer6
By Administrator on 2014-03-27T21:21:01 (imported from GitLab project)
By Administrator on 2014-03-27T21:21:01 (imported from GitLab)