<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, May 16, 2019 at 2:07 AM Martin Landa <<a href="mailto:landa.martin@gmail.com">landa.martin@gmail.com</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">Hi,<br>
<br>
st 15. 5. 2019 v 18:20 odesílatel Vaclav Petras <<a href="mailto:wenzeslaus@gmail.com" target="_blank">wenzeslaus@gmail.com</a>> napsal:<br>
> Another thing is the need for new contributing guidelines. Git is not Subversion and committing to master won't work (please, let me know if you want me to show some examples). What other OSGeo projects are doing is that contributing guidelines say that you should do pull request. It seems that this is often preferred way even for core developers. From what I gathered from a small sample of people at OSGeo sprint, the core devs don't go though fork, but they do go through a branch. In GitHub, we can set "Require pull request reviews before merging" and "Include administrators" for the master branch to enforce that. I think we should do it at least at the beginning.<br>
<br>
I tend to agree. Contributes with a write access should do PR for<br>
massive/tricky changes. It's perfect platform to discuss/improve<br>
changes before merging. Trivial changes can be committed/pushed<br>
directly, no PR needed.<br></blockquote><div><br></div><div>That's the tricky part. The direct commit-push workflow brings issues and projects tend to avoid it. The most direct access is (usually?) going through a branch in a main repo and merging that (merging through GitHub PR). The point of PR is not only discussion but also testing. The CI should run for every commit (chunk of commits in PR to be exact) before it goes into master.<br></div><div></div><div><br></div><div>Additionally, it there are simply no trivial changes. Even a change in documentation page can introduce an HTML tag not recognized by g.html2man causing compilation to fail. Similarly, with asynchronous commits you are at risk of conflicts and always in need of merge and the question is how you want this to be recorded in code history. With Subversion, each developer needs to resolve that before commit individually. With Git, you can have history with locally created automatic merge commits, you need do git pull --rebase locally, or you need to do PRs*.<br></div><div><br></div><div>I'm not saying this because I particularly like that, but because that's
 how Git/GitHub** is used and that's what we are signing up for.</div><div><br></div><div>* PRs are called merge requests in GitLab; perhaps hinting more on what they are for.<br></div><div>** This is not limited to GitHub, for example, GitLab by default locks master when you create a new repo.</div><div><br></div><div>Vaclav<br></div></div></div>