[sane-devel] Requiring GitLab merge requests for all changes

[Ralph Little]

Hi,

On 2022-01-01 6:56 p.m., Povilas Kanapickas wrote:
> On 12/31/21 12:36 AM, Ralph Little wrote:
>> Hi,
>>
>> On 2021-12-29 1:46 p.m., Povilas Kanapickas wrote:
>>> Hello,
>>>
>>> What do you think about requiring merge requests for all incoming
>>> changes on the sane-backends repository?
>>>
>>> This mostly doesn't change the current process because the majority of
>>> changes land to master via merge requests already. If the developer does
>>> not think a true review is necessary then he can merge the new merge
>>> request himself as soon as CI completes which currently introduces a
>>> delay of around 15 minutes.
>>>
>>> Out of 20 direct pushes to master since 1.0.32 we still managed to break
>>> master once (twice, if we count a MR merge without waiting for CI). This
>>> is important because any build failure will cause bisecting harder for
>>> unrelated backends.
>>>
>>> Lastly, merge requests provide a place for discussions even after a
>>> merge request is merged (e.g. if issues have been caused). Filing issue
>>> is not equivalent because there is no code review UI there.
>>>
>>> I can only think a single reasonable exception for the above policy: the
>>> push of the commit announcing a new release. With merge request we would
>>> need to create the release tag on a merge commit which is confusing.
>>>
>>> Please let me know what you think.
>>>
>>> Regards,
>>> Povilas
>> Personally, I always make changes to branches. I don't believe that
>> there should be functional changes direct to master, even for small
>> corrections.
>> I don't know if I would necessarily go all the way to require MRs for
>> every change but I am open to be convinced.
> I'm not sure I understood correctly, so instead of guessing, let's
> double check :) Are you not convinced that we should add a rule in
> GitLab that forbids direct pushes or are you against general policy?
I don't think we should permit direct commits to master as a rule, 
enforced either by policy or by some mechanism (if such a thing exists).
I'm not sure about requiring MRs for all branches though but I am open 
to persuasion.
I think that was my point.

Cheers,
Ralph