Probably my voice is not that important, but even though I'll share my point of view. For me it sounds very interesting. I seems like a chance for getting a good motivation for a very junior, like me, to get deeper into XWiki code and do it in regular mode.
So you have - my vote for this idea ;) Best, Krzysztof 2017-05-16 7:36 GMT+02:00 Sergiu Dumitriu <ser...@xwiki.org>: > Hi devs, > > A while ago GitHub introduced several features that I think can help > boost even more the code quality, reduce the bus factor, and make it > more attractive to contribute to the project. > > = Protected branches = > > Branches can be configured as protected in several ways: > * Require pull request reviews before merging > ** no direct commits are allowed, everything must go through a pull request > ** a pull request must be reviewed by at least one other trusted > committer before it is allowed to be merged > > * Require status checks to pass before merging > ** pull requests must pass automated checks, for example by being > successfully built by a CI service > > * Restrict who can push to this branch > ** official branches (master + stable-x.y) could be restricted to > approved committers, but we could grant write access to all other > interested developers more easily > > > = Reviews for pull requests = > > Comments on all commits as well on pull requests have been supported > since the early days of GitHub, but more recently it is easier to submit > (and request) actual reviews for pull requests. Instead of simply > commenting, and changing that special field on Jira, an official pull > request status is supported, with three states: > > - review required > - changes requested > - approved > > PR authors can either await for reviews from anybody, or request someone > in particular for a review. > > Committers can filter PRs by their status, and they can also show only > the PRs that require their review. > > > = Getting more eyeballs on the code = > > As Linus' Law states, "given enough eyeballs, all bugs are shallow". > We've been experimenting with mandatory pull requests for a while in my > other XWiki-based project, with great success so far. So here's a > proposal for a different development workflow: > > 1. Set up 2 teams, one with master (senior) committers, fully vetted > committers that have proven they understand the XWiki code, as well as > the XWiki product, and one with junior committers > 2. Protect the master branch. Require pull requests and reviews before > any code is merged in it. Restrict push only to the master committers. > 3. Everybody works on separate branches > 4. Once the code is almost done, make a pull request and require reviews > from two junior committers > 4a. Or, instead of requesting, let committers volunteer themselves, but > it's less likely that someone will volunteer on time > 5. The junior committers must then carefully review the code, and either > require changes, just comment, or approve the pull request > 6. Once the junior committers have approved the pull request, the PR > author must request review from a senior committer > 7. The senior committer can either approve and merge the pull request, > or send it back to step 3. > e. We can define allowed exceptions: don't require PRs from senior > committers if the fixes are really small and obvious, like fixing typos, > or small and important bugfixes that must be merged quickly, but these > should really be rare exceptions > > While it does sound like more work for an already overworked team, this > has many benefits: > * the code will have better quality > * awareness of: > ** what's new / changing > ** how others are approaching a problem (especially juniors learning > from seniors by being exposed to more code) > ** the existing code, since the codebase is large and otherwise people > have few occasions to look at many of the parts of XWiki > * this means a larger bus factor for new code, and slowly increasing it > for existing code that's being touched by one and reviewed by many > * theoretically, less time spent doing reviews, since all committers > should look over every commit anyway, but this way they are explicitly > told when they should look, instead of wasting time reviewing work in > progress > * larger community, since people can more quickly become junior > committers instead of having to invest many months of years of forkwork > before committership > * easier collaboration on code, since it's very hard to work on someone > else's fork branches, but easy to work on an origin branch > > > So, what do you think? > -- > Sergiu Dumitriu > http://purl.org/net/sergiu/ >
