Once a merge requests can be made, #3071752: Enable ability to open merge requests for branches in issue forks, they will eventually be ready for commit.
GitLab’s UI exists and is hopefully useful. Drupal.org’s issue UI handles credits in commit message drafting.
We should also make sure both regular merges and squash merges work well.
If this is done via the GitLab UI, something like #2270763: Automatically fix issues when commit message contains "Fixed …" keyword (powerful/smart commit messages) would remove an extra bit of work.
Issue fork drupalorg-3072047
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
drummComment #3
MixologicWe need to make sure we understand how https://docs.gitlab.com/ee/user/discussions/index.html#merge-request-rev... workflow would impact the information we bring back to drupal.org
Comment #4
drummThere are a few options which we might want to make configurable per-project:
For the initial enabling of merge requests, I’m setting the merge method to fast forward, to match the patch workflow. There are two other options at https://docs.gitlab.com/ee/api/projects.html#project-merge-method
The option to squash or not is chosen by the project maintainer at merge time in GitLab’s UI. The person opening the merge request can make a suggestion. Once we upgrade to GitLab 13.2, there are new options: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33827
We could also add the
only_allow_merge_if_all_discussions_are_resolved
option.Comment #5
drummGitLab 13.3 added squash commit options to allow/require/etc squash merges per-project https://about.gitlab.com/releases/2020/08/22/gitlab-13-3-released/#squas...
Comment #6
drummThere is not an API for setting that per-project https://gitlab.com/gitlab-org/gitlab/-/issues/238531
Comment #8
volegerHow credits (commit message) from the issue page would be populated to the merge request form?
Also, to mimic the patch flow every merge requests must use squash merging. Can squash be set as default for the new merge requests?
Comment #9
drummGitLab does have a setting squash merges to be the default per-project, which we could set for every project. Except that it is not available in any APIs: https://gitlab.com/gitlab-org/gitlab/-/issues/238531.
We can update every merge request as it is opened, and maybe ongoing, with https://docs.gitlab.com/ee/api/merge_requests.html#update-mr. That may be useful if we want to force merge request titles or descriptions to have some standard content, or something else like that.
I fully expect we’ll end up building our own merge button on the issue page, to be able to always use our commit message.
Comment #10
voleger+1 for the way to somehow merge requests from the issue page
Comment #12
drummAll new merge requests will now be updated to default to squash merge, if that isn’t already set.
I’ve also bulk updated all open merge requests to default to squash merge, and default to not deleting the source branch. Since we do not allow issue fork branch deletion, deleting the source branch will never work.
Comment #13
drummGitLab does not quite squash commits, which would follow the patch commit style: https://gitlab.com/gitlab-org/gitlab/-/issues/1822
GitLab can get to the same result with a fast-forward squash merge. Within GitLab, that is implemented as first rebasing. For issue forks, we do not want people overwriting each others’ commits, so we disallow force pushes, which also disallows most rebases (we can detect and allow some simple rebases).
Manually pushing a squash commit does not mark the merge request as merged. GitLab does not have a way to detect if an arbitrary commit is actually merging in changes from a merge request.
To build our own merge button on the issue queue, there are a couple options to work around this:
I’m leaning toward the second option. This lets people rewrite history as they might expect to do, but won’t lose the previous commits. And it is a less-large workaround.
Comment #14
drummhttps://bitbucket.org/drupalorg-infrastructure/drupal-git-hooks/commits/... works well for allowing force pushes.
https://docs.gitlab.com/ee/api/README.html#sudo works well with the credentials that Drupal.org uses. This lets the merge be done by the project maintainer.
Putting this all together for implementation should be straightforward.
Comment #15
drummThe Author radio buttons will not do anything for merging merge requests when this launches, since GitLab’s API does not have a way to set a separate merge commit author. #3177220: Remove commit author radios in the "Credit & committing" section. is the issue for resolving that UX.
Comment #16
volegerHow about to provide the optional way to merge request manually by building the code snipped based on the selected merge request:
Or even what about to hangle this workaround in the separate endpoint at some drupal.org service to mimic merging request with setting proper authoring of the merge commit untill it would be possible to handle that with GitLab API?
Comment #17
drummI tested out this workflow, and GitLab does indeed not detect that the merge request is merged. I could not find a way to mark it as merged, so closing is indeed the next best.
In general, I hope to lean on GitLab as much as possible and keep our integration lightweight. If we did build this out, it would have the same closed end state for the merge request instead of merged, although that’s not too big of a deal.
Comment #21
drummMaintainers now have a select menu at the bottom of the Credit & committing fieldset to merge a mergeable merge request with the templated commit message.