<div dir="ltr"><div>I thought I'd reanimate this discussion to see if I could get a little help. I've been working on SANE for many years, even before we used git. I've been committing and pushing to master from the beginning. I pretty much use the command line exclusively. I've never had a problem I could not recover from. But, I'm not well versed in gitlab. So, I need a cheat sheet for how you guys think changes should be committed now. Something simple that an old unix graybeard can handle :)</div><div><br></div><div>thanks for your help-<br></div><div>allan<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jan 3, 2022 at 4:29 PM Povilas Kanapickas <<a href="mailto:povilas@radix.lt">povilas@radix.lt</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 1/2/22 6:30 PM, Ralph Little wrote> On 2022-01-02 3:14 a.m., Povilas<br>
Kanapickas wrote:<br>
>> On 1/2/22 5:07 AM, Ralph Little wrote:<br>
>>>>> Personally, I always make changes to branches. I don't believe that<br>
>>>>> there should be functional changes direct to master, even for small<br>
>>>>> corrections.<br>
>>>>> I don't know if I would necessarily go all the way to require MRs for<br>
>>>>> every change but I am open to be convinced.<br>
>>>> I'm not sure I understood correctly, so instead of guessing, let's<br>
>>>> double check :) Are you not convinced that we should add a rule in<br>
>>>> GitLab that forbids direct pushes or are you against general policy?<br>
>>> I don't think we should permit direct commits to master as a rule,<br>
>>> enforced either by policy or by some mechanism (if such a thing exists)<br>
>> Gitlab allows adding a per-branch rule that has 3 settings:<br>
>>   - Allowed to merge (Developers+Maintainers / Maintainers / No one / A<br>
>> specific set of users)<br>
>>   - Allowed to push (Developers+Maintainers / Maintainers / No one / A<br>
>> specific set of users)<br>
>>   - Allowed to force push (Yes/No)<br>
>><br>
>> So for example we could configure the following for master branch:<br>
>><br>
>>   - Allowed to merge = Developers+Maintainers<br>
>>   - Allowed to push = No one (this would be relaxed to Maintainers for<br>
>> the release commit)<br>
>>   - Allowed to force push = No<br>
>><br>
>> Myself I slightly prefer the above strict rule because pushes of wrong<br>
>> branch to wrong remote happen to everyone. An agreement would work<br>
>> fine too.<br>
>><br>
>>> I'm not sure about requiring MRs for all branches though but I am open<br>
>>> to persuasion.<br>
>>> I think that was my point.<br>
>> I'm still not sure I understand :-) Are you talking about requiring a MR<br>
>> from any arbitrary branch abc to another branch xyz? Or about requiring<br>
>> a MR from any arbitrary branch abc to "master" branch? Could you please<br>
>> explain like I'm five with a couple examples?<br>
>><br>
> I guess my point is that I don't really care much about requiring Merge<br>
> Requests for any changes to the repo as much as I care about not pushing<br>
> changes directly to master without branching and merging (and benefiting<br>
> from our pipeline checks).<br>
> It is too easy to break master by a careless single push.<br>
<br>
I finally understood. I was not thinking about the third option of not<br>
using merge requests, but just using separate branches, checking the CI<br>
results and merging via local git instance.<br>
<br>
> Referring to your original proposition:<br>
> <br>
>> What do you think about requiring merge requests for all incoming<br>
>> changes on the sane-backends repository?<br>
> <br>
> ... I am open to discussion certainly and I can see the advantages.<br>
<br>
Making sure the changes work via pushing branches and checking CI solves<br>
the issue of broken commits being pushed just like requiring merge requests.<br>
<br>
In addition to the above, requiring merge requests solve this:<br>
 - there's a place to conveniently comment about any change landing to<br>
the master branch far into the future<br>
 - the chance of accidental push to master is eliminated (happened to<br>
even to me, though on a different project)<br>
<br>
I don't know whether these are important points to convince you. Myself<br>
I only have slight preference for requiring merge requests and Wolfram<br>
indicated that he strongly supports it due to ability to do code reviews.<br>
<br>
Cheers,<br>
Povilas<br>
<br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr" class="gmail_signature">"well, I stand up next to a mountain- and I chop it down with the edge of my hand"</div>