Add GitLab merge request template to adblockpluscore repo

[mjethani]

Background

Since we have started doing code reviews on GitLab, it makes sense to have at least one default ​merge request template.

See the GitLab documentation for ​description templates.

What to change

Add a file .gitlab/merge_request_templates/default.md to the repo with an appropriate template.

[sebastian]

I'm curious what you would put into a merge request template?

[mjethani]

Replying to sebastian:

I'm curious what you would put into a merge request template?

Frankly I have been struggling with it and at this point I think maybe it's even a good idea to let it evolve.

[mjethani]

Anyway, since you asked, the initial template I wrote had this:

• List of issues affected
• Description: What you have changed, what the original problem is, how you are addressing it in your patch, and so on
• Checklist: At a minimum, your patch must address the issue(s), pass all tests, and pass linting; beyond this, it should update any related documentation and tests, and it should work with adblockpluschrome; if it does not work out of the box with adblockpluschrome, please explain how you broke compatibility and what adblockpluschrome or any of its dependencies can do to catch up; if your patch affects performance (memory or CPU usage), please explain how
• How important is this patch to you personally? (e.g. the changes would be very valuable for the next release, which is going into code freeze soon, and you are off on vacation in a few days; in that case, please say "High" priority so the reviewers know to look at this first; but also do not abuse this)

At some point I thought that it was a bit overdone and I left it.

Nevertheless, I have started doing something like this for my patch submissions (e.g. ​!43, ​!45, ​!47, ​!48, ​!51).

I think we should try different ideas and let it evolve.

[mjethani]

Replying to mjethani:

• Description: What you have changed, what the original problem is, how you are addressing it in your patch, and so on

This, by the way, is inspired by the commit messages for my Chromium changes. I would briefly describe the issue at a technical level and what the patch did to address it. e.g. ​https://crrev.com/c/913628 (review) and ​https://crrev.com/538722 (commit).

[sebastian]

Thanks for sharing. However, most of these information seem redundant to me. The issues addressed should be referenced in the commit message / merge request title, information about the change itself should rather go into the issue description (maybe except for some optional low level explanations), and making sure that tests and linting passes is done through the CI.

Also I doubt that prioritization through the merge request description will work. I mostly work through my inbox like a TODO list. When I get to the notification email of the merge request, I will review it, but I won't read the merge request description any earlier. When I have some backlog and there is an urgent merge request you might have to ping me on IRC, and I'm used to do the same when I feel that a merge request that I created isn't getting enough attention. The exception might be huge patches that take me several hours to review, but I won't get around to review the merge request any sooner just because someone requested a priority review either in that case.

[sebastian]

Sorry, but we switched to GitLab. If this issue is still relevant, please file it again in the ​new issue tracker.