Gerrit Code Review System

[ckwalsh]

One of the problems I see with the current development process is that users do not feel like their patches/code are actually being considered, and instead are just being thrown away.

I propose using the Gerrit code review software Google has developed. It allows anyone to contribute, and the code does not actually get merged in until a developer sign off. It also allows for annotations.

It seems to fill all the goals that the Junior Developers group was designed to reach, without the selective procedure.

[naderman]

I've added a few of my general thoughts about how we should use git here http://wiki.phpbb.com/PhpBB4/Developmen ... ucture#git That is certainly still incomplete though.

[ckwalsh]

Patches from regular users can be picked up by any other user or developer. They should be audited closely. Since they also need to go through multiple layers until they reach the release manager quality concerns should be satisfied.

That still requires users to shop for a developer willing to take their patch. It also ruins much of the ownership feeling. It is much more satisfying to see a commit you made as the author in a repository, rather than "naderman: patch supplied by Brainy". Sure, it's a small thing, but a feeling of ownership ("I contributed, look and see") will certainly cause more users to participate.

In addition, while code is audited by a developer, it is not made specifically available and out in the open. The big advantage of something like Gerrit is that everyone can see suggestions and comment/make suggestions, then have the patch be approved. It is completely transparent to all, rather than a single developer.

Perhaps I am completely missing your idea though.

Setting up a system like github.com on code.phpbb.com might make sense to allow developers (maybe even regular users) to set up their public repositories on phpbb.com.

Sounds cool

[naderman]

Patches from regular users can be picked up by any other user or developer. They should be audited closely. Since they also need to go through multiple layers until they reach the release manager quality concerns should be satisfied.

That still requires users to shop for a developer willing to take their patch. It also ruins much of the ownership feeling. It is much more satisfying to see a commit you made as the author in a repository, rather than "naderman: patch supplied by Brainy". Sure, it's a small thing, but a feeling of ownership ("I contributed, look and see") will certainly cause more users to participate.

Through the way git works you actually keep the information about the author etc. in the history, afterall it's distributed. You don't commit someone elses changes, you commit the merge which keeps history of all merge parents. So there's no issue like that which one has in svn.

In addition, while code is audited by a developer, it is not made specifically available and out in the open. The big advantage of something like Gerrit is that everyone can see suggestions and comment/make suggestions, then have the patch be approved. It is completely transparent to all, rather than a single developer.

Perhaps I am completely missing your idea though.

Since every developer and user has their own public repository it's of course out there in the open. Available for anyone to merge into their own repository. Afterall that's how a development team member would get a user's code. From their public git repository (not a patch file).

And apart from that I haven't looked at Gerrit yet, that might still be useful on top of this basic git usage.

[ckwalsh]

Through the way git works you actually keep the information about the author etc. in the history, afterall it's distributed. You don't commit someone elses changes, you commit the merge which keeps history of all merge parents. So there's no issue like that which one has in svn.

Ah, I didn't realize everyone would have their own repository

Since every developer and user has their own public repository it's of course out there in the open. Available for anyone to merge into their own repository. Afterall that's how a development team member would get a user's code. From their public git repository (not a patch file).

And apart from that I haven't looked at Gerrit yet, that might still be useful on top of this basic git usage.

What system would there be to alert developers "Hey, I have a patch, come look at it!"? That I guess is what I am really wondering about.

And I meant to link to Android's gerrit installation.

[code reader]

.........
Since every developer and user has their own public repository it's of course out there in the open.
.....

well, this is not exactly how Git works.
while it's true that with git, (as with any other DVCS), everyone (the distinction between users and developers is irrelevant here) has their own repo, this does not necessarily mean it's public.
usually people keep and use their *private* repos, and some may choose to also maintain a public repo.
typically, you do not "use" the public repo - you "publish" stuff from your private repos to the public one when you choose.
(i said "repos" rather than repo, because typically you will have *at least* one repo per machine you use, and some people have more than one machine)

with Git, you can communicate changes in several ways:
-- you can "push" changes. this is the typical communication between a user and her "public repo". for instance, i pull changes from SVN into my private Git repo, and then push it to the public repo (see my sig).
-- you can "pull" changes (e.g., i could ask naderman to "pull branch X from my public repo")
-- you can exchange patches. with Git, the meta-information such as authorship, "signed-of-by", "ack-by", "tested-by" etc. is preserved when using patches, so if i use git to create a patch, publish it (say, on area51, within "code" clause) and nademan would take the patch and apply it *as is*, it will appear on *his* repo as *my* commit. neat, ha?

this mode allows me to create a patch in "RFC" mode. people would then comment both on the desirability of the change, the global design, and specific coding/style/documentation etc.
i would then publish successive refinements of the patch (presumably all within the same thread).
once a consensus is reached that the change is indeed desirable and implemented well, and hopefully with one or more people who actually tried the change and confirm it does what it supposed to do and has no ill effects, then the maintainer would be able to take the patch (as easy as cut-n-paste) into his or her tree, where it will appear under my name, with all the "s-o-b", "acked-by", "tested-by", maybe a link to the issue tracker etc.
once the maintainer merge this commit into the "mainline" branch, then the next time the maintainer will push his repo to the public one, this change will appear in the public repo.


one of the neat things here is that everyone can play maintainer (except the last part of pushing to the "canonical" public repo), so if i post a patch, you will be able, immediately, to pull it into your own repo and test it, so even if you do not write php code yourself you will be able to actively participate in the development process as tester (not like today where you can test things after they are already in the canonical tree: here "testing" means testing the RFC patch).
development of documentation, unit-tests and everything else follows the same path as development of code: you publish an RFC documentation patch, people comment, you refine, consensus is reached and the change get merged into upstream.

the maintainer herself might choose to publish a patch in hope of getting useful feedback that would help making this patch better before promoting it to the "canonical" branch. of course, being the maintainer (s)he can just push changes to the public tree before anyone else sees them, but in some projects this is just not done.
e.g., Linus will never push a change to the linux tree: he will *always* publish it to the linux-kernel mailing list first, and only after giving people the chnce to comment (he can, and often does, choose to ignore comments he thinks are stupid) will this be merged into the "linus tree" which is the public canonical tree of the linux kernel.
and btw: the "linus tree" is canonical only by social convention, i.e. the fact that people treat it so. there is no technical distinction between this tree and any other tree.

in a sense, you can look at Git as a system where *every repo is a fork*, with very good tools to transfer branches and specific commits from one fork to another.

Gerrit (i have no experience with it) is built as a more structured way to do code reviews on top of git.
as far as i understand it, Gerrit also integrate with some issue tracking system.
i do not know if Gerrit is a good match for phpbb, but as long as you made up your mind that you want to migrate your development process from the "cathedral" model to the "bazaar" model, a DVCS is a very natural choice, and Git is a good choice of DVCS (mercurial would have also been fine).


peace.

[naderman]

Actually that's kind of what I meant, thanks for writing it down in more detail I was thinking of "everyone has a public repo" in the github sense. Obviously you could just post patches instead and you have as many private repos as you like, other people just don't usually see them

Anyway, what you're describing is the development model at least I had in mind when making the decision to switch to git.