A reminder that a huge added bonus is that going through PRs gives us thechance to setup automated CI builds, which would allow us to know if we are breaking linux building or such, *before *we merge.
On 17 September 2017 at 19:32, Colin Finck <co...@reactos.org> wrote: > With two votes in favor of a Pull Request policy and no objections here > or on IRC, I'm going to restrict our GitHub to only allow contributions > to the "master" branch using Pull Requests. Although Pull Requests of > trusted committers won't require a review beforehand. Nevertheless, you > are encouraged to do that for bigger changes. > > This is your last chance to object! > I know that enforcing Pull Requests adds another step until your code > makes it into master. On the other hand, they pave the way for frequent > code reviews and ensure a cleaner master branch. > > Finally, don't forget that you will most likely work in your own > branches anyway. These come without any restrictions and give you > independence from other people's changes to master. You can even modify > your own commits after pushing. > Hopefully, this fosters the "commit early, commit often" idea and losing > code in working copies on broken HDDs becomes a thing of the past. > > Cheers, > > Colin > > > Am 14.09.2017 um 19:07 schrieb Thomas Faber: > > I'm wary of opening up pushing to master. If we really want a linear > > history, it should be enforced. Accidents with version control happen > > all the time (when was the last time the SVN pre-commit hook stopped you > > from committing because you forgot to set eol-style?). > > > > I wouldn't mind forcing the use of pull requests, assuming we can set it > > up so that merge is allowed with no reviews (those would be recommended, > > but enforcing them seems too much). > > > > Though I'm also okay with giving up the linear history idea altogether, > > or hosting the repo elsewhere (e.g. we could host a Bitbucket instance). > > > > > > On 2017-09-14 17:48, David Quintana (gigaherz) wrote: > >> I vote for recommending devs to use pull requests, and to make it a > >> semi-strict policy that devs who push directly to master should ALWAYS > >> make > >> sure to pull and rebase before pushing. > >> > >> I myself intend on almost exclusively contribute through pull requests. > >> This means: > >> > >> 1. I'll push to my own fork, and work backed by the fork > >> 2. When I'm done doing something, I will use the pull request > >> feature, > >> rather than "git push upstream/master" > >> 3. If the changes are non-trivial, I'll ask for a second opinion > from > >> some other developer > >> 4. I'll push "merge with rebase" myself, but only when I feel the > >> changes are "sufficiently approved" > >> > >> I would vote for using such a workflow as a primary way of contributing, > >> since it avoids all sorts of issues, and has the added benefit of the > >> automatic "merge with rebase". > >> > >> On 14 September 2017 at 17:23, Colin Finck <co...@reactos.org> wrote: > >> > >>> Hi all! > >>> > >>> After playing around with GitHub's features in > >>> https://github.com/colinfinck/sandbox for the last few days, it turns > >>> out that its server-side settings hardly prevent repository mess. > >>> > >>> Although I have set master to be a "protected branch" and enabled the > >>> option "Require branches to be up to date before merging", it allows me > >>> to push just everything that doesn't rewrite history. This includes any > >>> kind of merge commits, even the nasty automatic merges that occur when > >>> you commit to your outdated local master and then do the requested "git > >>> pull" before pushing. > >>> GitHub Staff confirmed to me that GitHub has no way of enforcing a > >>> rebase-only workflow. For a self-hosted Git, a simple pre-receive hook > >>> like https://stackoverflow.com/a/5493549 would do the trick. > >>> > >>> The only other option GitHub offers is requiring each commit to go to a > >>> branch. Changes to master can only happen through an approved Pull > >>> Request then. This would drastically change our workflow though, and I > >>> don't think we want this additional burden for every minor change. > >>> > >>> Before we now reverse our decision for a GitHub master repo, let's > >>> verify that a self-hosted Git repo with an enforced rebase-only > workflow > >>> is really what we want: > >>> > >>> - It would ensure a linear history in the "master" branch, just like > >>> WINE has. If you sort by "Commit Date" instead of "Author Date", the > >>> history would even be chronological. > >>> - When contributing changes from a branch back to "master" using "git > >>> rebase", every single commit is reapplied with a new hash. Except for > >>> the author dates, these commits have no link to the original branch. > >>> - Without GitHub as the master repo, we would lose the ability to merge > >>> Pull Requests directly from the website. That would sooner or later > turn > >>> the entire Pull Request feature of GitHub useless for us. > >>> Contrary to what people told me, GitHub doesn't detect when you merge > >>> the Pull Request locally and push these changes back to GitHub. > >>> > >>> > >>> On the other hand, what would an unrestricted GitHub provide us: > >>> > >>> - Discussing, improving, and later merging Pull Requests directly on > the > >>> website. Merging, squash merging, and rebase merging is supported. Not > >>> just for our own branches, but also for forks of the ReactOS repo. > >>> - Our history graph in the "master" branch will inevitably become a > >>> stream of parallel lines, making it harder to follow the history > >>> chronologically. Worst-case example is the Linux kernel: > >>> http://fs5.directupload.net/images/170914/h5pddxx7.png > >>> (ok, admittedly gitk renders this graph a bit nicer) > >>> > >>> Allowing merges used to be an even bigger problem when we still wanted > a > >>> linear history to replicate SVN revision numbers. But we now replaced > >>> revision numbers by the output of "git describe". So maybe merge > commits > >>> are ok now? > >>> At least, we should be able to prevent automatic merge commits by > >>> instructing every committer to use "git fetch && git rebase origin" > >>> instead of "git pull" when syncing with the server. A nicely written > and > >>> illustrated (Tortoise)Git guide on the Wiki should do the job :) > >>> > >>> > >>> I tend to favor the second option right now and allow merges, but I > >>> definitely need more input on this from you. > >>> > >>> > >>> Best regards, > >>> > >>> Colin > > > > _______________________________________________ > > Ros-dev mailing list > > Ros-dev@reactos.org > > http://www.reactos.org/mailman/listinfo/ros-dev > > _______________________________________________ > Ros-dev mailing list > Ros-dev@reactos.org > http://www.reactos.org/mailman/listinfo/ros-dev >
_______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
