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
