Our team is growing, I felt the need to create some patch submission guidelines, so we all know how to cooperate efficiently. I put it here:
https://fedoraproject.org/wiki/AutoQA_Patch_Process#AutoQA_developers
The main concept is that we are a small team with a lot of work on our plate, so it's better to possibly revert some bad patches rather then force people to wait for some approvals. The technology (git) allows us to make (and revert) mistakes, so let's make use of that and eliminate bureaucratic processes. I definitely don't want to be a frog on a spring (Czech saying) that must approve every single proposed patch before pushing it into master. Nope, I know better than that - you're all responsible for that! :-)
This partly relates to the recent Tim's work - I still didn't have time to go through that, I'm sorry. But if anyone else agrees it looks good, then it probably looks good. And that applies to everyone's work. Of course, using common sense is strongly advised in all possible circumstances :-)
Good/bad?
Thanks, Kamil
On 03/09/2011 06:38 AM, Kamil Paral wrote:
Our team is growing, I felt the need to create some patch submission guidelines, so we all know how to cooperate efficiently. I put it here:
https://fedoraproject.org/wiki/AutoQA_Patch_Process#AutoQA_developers
I like the proposal, it's a good balance between the level of effort required for processes and potential chaos/loss of code quality.
I'm kind of a stickler for code quality and a firm believer that my own code is the most difficult to see issues with. I like to have code reviews for non-trivial changes even though they can be a bit of a pain.
The main concept is that we are a small team with a lot of work on our plate, so it's better to possibly revert some bad patches rather then force people to wait for some approvals. The technology (git) allows us to make (and revert) mistakes, so let's make use of that and eliminate bureaucratic processes. I definitely don't want to be a frog on a spring (Czech saying) that must approve every single proposed patch before pushing it into master. Nope, I know better than that - you're all responsible for that! :-)
I agree 100% that there should not be a single person who is responsible for patch approval. It just creates a bottleneck and one unfortunate soul who is spending a disproportionate amount of time reviewing everyone else's code.
With that in mind, I'll try to start participating more in patch review but for now, at least, I'm likely to defer some judgement to people with more experience in Fedora QA.
This partly relates to the recent Tim's work - I still didn't have time to go through that, I'm sorry. But if anyone else agrees it looks good, then it probably looks good. And that applies to everyone's work. Of course, using common sense is strongly advised in all possible circumstances :-)
Personally, I tend to err on the side of caution when I'm new to a project. I'm reading this as a suggestion (not just pointed at me) to be a little less timid about patches and merges. Am I understanding correctly?
Good/bad?
Is there any interest in trying something like Review Board [1]? It was inspired by a talk by Guido van Rossum on code reviews at google [2] a while back. The actual tool that came out of that talk [3] was designed for google app engine, which I would rather avoid if we can but would be up for trying it if there is interest.
Tim
[1] http://www.reviewboard.org/ [2] http://video.google.com/videoplay?docid=-8502904076440714866# [3] http://code.google.com/p/rietveld/
On Wed, 2011-03-09 at 10:24 -0700, Tim Flink wrote:
On 03/09/2011 06:38 AM, Kamil Paral wrote:
Our team is growing, I felt the need to create some patch submission guidelines, so we all know how to cooperate efficiently. I put it here:
https://fedoraproject.org/wiki/AutoQA_Patch_Process#AutoQA_developers
I like the proposal, it's a good balance between the level of effort required for processes and potential chaos/loss of code quality.
I'm kind of a stickler for code quality and a firm believer that my own code is the most difficult to see issues with. I like to have code reviews for non-trivial changes even though they can be a bit of a pain.
The main concept is that we are a small team with a lot of work on our plate, so it's better to possibly revert some bad patches rather then force people to wait for some approvals. The technology (git) allows us to make (and revert) mistakes, so let's make use of that and eliminate bureaucratic processes. I definitely don't want to be a frog on a spring (Czech saying) that must approve every single proposed patch before pushing it into master. Nope, I know better than that - you're all responsible for that! :-)
I agree 100% that there should not be a single person who is responsible for patch approval. It just creates a bottleneck and one unfortunate soul who is spending a disproportionate amount of time reviewing everyone else's code.
With that in mind, I'll try to start participating more in patch review but for now, at least, I'm likely to defer some judgement to people with more experience in Fedora QA.
This partly relates to the recent Tim's work - I still didn't have time to go through that, I'm sorry. But if anyone else agrees it looks good, then it probably looks good. And that applies to everyone's work. Of course, using common sense is strongly advised in all possible circumstances :-)
Personally, I tend to err on the side of caution when I'm new to a project. I'm reading this as a suggestion (not just pointed at me) to be a little less timid about patches and merges. Am I understanding correctly?
Good/bad?
Is there any interest in trying something like Review Board [1]? It was inspired by a talk by Guido van Rossum on code reviews at google [2] a while back. The actual tool that came out of that talk [3] was designed for google app engine, which I would rather avoid if we can but would be up for trying it if there is interest.
It was mentioned a long time ago, and I think it is now available for use in Fedora.
https://admin.fedoraproject.org/pkgdb/acls/name/ReviewBoard https://fedoraproject.org/wiki/ReviewBoard_Infrastructure_SOP https://fedorahosted.org/reviewboard
I don't know how a hosted project takes advantage of it though, perhaps someone in #fedora-admin might be able to help?
Thanks, James
On 03/09/2011 11:23 AM, James Laska wrote:
Is there any interest in trying something like Review Board [1]? It was inspired by a talk by Guido van Rossum on code reviews at google [2] a while back. The actual tool that came out of that talk [3] was designed for google app engine, which I would rather avoid if we can but would be up for trying it if there is interest.
It was mentioned a long time ago, and I think it is now available for use in Fedora.
https://admin.fedoraproject.org/pkgdb/acls/name/ReviewBoard https://fedoraproject.org/wiki/ReviewBoard_Infrastructure_SOP https://fedorahosted.org/reviewboard
I don't know how a hosted project takes advantage of it though, perhaps someone in #fedora-admin might be able to help?
Thanks, James
I didn't realize that was available to us and was going to set up a demo instance on my own.
I'll look into it, looks like it is a matter of one of us getting sufficient privileges to add a repo to the existing review board instance.
Tim
On 03/09/2011 01:48 PM, Tim Flink wrote:
On 03/09/2011 11:23 AM, James Laska wrote:
Is there any interest in trying something like Review Board [1]? It was inspired by a talk by Guido van Rossum on code reviews at google [2] a while back. The actual tool that came out of that talk [3] was designed for google app engine, which I would rather avoid if we can but would be up for trying it if there is interest.
It was mentioned a long time ago, and I think it is now available for use in Fedora.
https://admin.fedoraproject.org/pkgdb/acls/name/ReviewBoard https://fedoraproject.org/wiki/ReviewBoard_Infrastructure_SOP https://fedorahosted.org/reviewboard
I don't know how a hosted project takes advantage of it though, perhaps someone in #fedora-admin might be able to help?
Thanks, James
I didn't realize that was available to us and was going to set up a demo instance on my own.
I'll look into it, looks like it is a matter of one of us getting sufficient privileges to add a repo to the existing review board instance.
Tim
I requested that AutoQA be added to the fedorahosted ReviewBoard instance. We should have access to that sometime in the next week. However, I have been warned that: 1) Email notifications don't work 2) Performance is abysmal
This may improve eventually, but it sounds like the integration with FAS has been delayed for now.
Tim
Personally, I tend to err on the side of caution when I'm new to a project.
I completely understand that.
I'm reading this as a suggestion (not just pointed at me) to be a little less timid about patches and merges. Am I understanding correctly?
Basically we haven't really had our patch review process defined well. I didn't know personally whether I'm supposed to review all patches or not. Whether you are supposed to wait for it. I could imagine that after sending the patch you have asked yourself "so what am I waiting for next?". Etc. So I wrote up and proposed the guidelines. It doesn't have to be followed precisely, no hard rules in here.
So be as timid as you like :-) OTOH if you see committing your patch to master like a no-brainer, you can just do it.
autoqa-devel@lists.fedorahosted.org