Have you guys considered gerrit code review for patches? as well as setting up build bots to test build the patches?
On Tue, Apr 16, 2013 at 3:59 PM, Jonathan Aquilina eagles051387@gmail.com wrote:
Have you guys considered gerrit code review for patches? as well as setting up build bots to test build the patches?
which projects at Linaro you have in mind for this request? it's already being used for Android 'stuff', see
Wouldnt all of them benefit in a way from it?
On Tue, Apr 16, 2013 at 4:04 PM, Nicolas Dechesne < nicolas.dechesne@linaro.org> wrote:
On Tue, Apr 16, 2013 at 3:59 PM, Jonathan Aquilina eagles051387@gmail.com wrote:
Have you guys considered gerrit code review for patches? as well as
setting
up build bots to test build the patches?
which projects at Linaro you have in mind for this request? it's already being used for Android 'stuff', see
On 16 April 2013 07:06, Jonathan Aquilina eagles051387@gmail.com wrote:
Wouldnt all of them benefit in a way from it?
My take on this is that one of the goals of Linaro's assignee process is to teach folks how to work in the open source community and than take that knowledge back to internal teams. This means we should use communication methods and tools that are native to the upstream communities we interact with and in the case of the kernel, this means that code review should be done via email lists.
~Deepak
On Tue, Apr 16, 2013 at 4:04 PM, Nicolas Dechesne nicolas.dechesne@linaro.org wrote:
On Tue, Apr 16, 2013 at 3:59 PM, Jonathan Aquilina eagles051387@gmail.com wrote:
Have you guys considered gerrit code review for patches? as well as setting up build bots to test build the patches?
which projects at Linaro you have in mind for this request? it's already being used for Android 'stuff', see
-- Jonathan Aquilina
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
-- Deepak Saxena - Kernel Working Group Lead Linaro.org | Open source software for ARM SoCs Follow Linaro: http://www.facebook.com/pages/Linaro
Doesnt email run the risk of a patch slipping through the cracks?
On Tue, Apr 16, 2013 at 9:02 PM, Deepak Saxena dsaxena@linaro.org wrote:
On 16 April 2013 07:06, Jonathan Aquilina eagles051387@gmail.com wrote:
Wouldnt all of them benefit in a way from it?
My take on this is that one of the goals of Linaro's assignee process is to teach folks how to work in the open source community and than take that knowledge back to internal teams. This means we should use communication methods and tools that are native to the upstream communities we interact with and in the case of the kernel, this means that code review should be done via email lists.
~Deepak
On Tue, Apr 16, 2013 at 4:04 PM, Nicolas Dechesne nicolas.dechesne@linaro.org wrote:
On Tue, Apr 16, 2013 at 3:59 PM, Jonathan Aquilina eagles051387@gmail.com wrote:
Have you guys considered gerrit code review for patches? as well as setting up build bots to test build the patches?
which projects at Linaro you have in mind for this request? it's already being used for Android 'stuff', see
-- Jonathan Aquilina
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
-- Deepak Saxena - Kernel Working Group Lead Linaro.org | Open source software for ARM SoCs Follow Linaro: http://www.facebook.com/pages/Linaro
On Wed, Apr 17, 2013 at 6:21 AM, Jonathan Aquilina eagles051387@gmail.com wrote:
Doesnt email run the risk of a patch slipping through the cracks?
as highlighted by Deepak above, each project will use the processes/methods inherited from its 'upstream'. That's why gerrit is being used for Android activities for example. many successful open source projects are using solely email for patch submission and review, and that happens to work fine. yes, that makes lots of emails, and patches can be lost, but the general rule is that it's up to the patch author to follow-up and potentially resubmit when such things happen. for example, the Linux kernel documentation says this:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documen...
nicolas
Totally under stand :). Hopefully come summer I will be able to get my hands on a board and start contributing :)
On Wed, Apr 17, 2013 at 9:51 AM, Nicolas Dechesne < nicolas.dechesne@linaro.org> wrote:
On Wed, Apr 17, 2013 at 6:21 AM, Jonathan Aquilina eagles051387@gmail.com wrote:
Doesnt email run the risk of a patch slipping through the cracks?
as highlighted by Deepak above, each project will use the processes/methods inherited from its 'upstream'. That's why gerrit is being used for Android activities for example. many successful open source projects are using solely email for patch submission and review, and that happens to work fine. yes, that makes lots of emails, and patches can be lost, but the general rule is that it's up to the patch author to follow-up and potentially resubmit when such things happen. for example, the Linux kernel documentation says this:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documen...
nicolas
On Wed, 2013-04-17 at 06:21 +0200, Jonathan Aquilina wrote:
Doesnt email run the risk of a patch slipping through the cracks?
And with gerrit the patch author needs to get an account enabled with the project, produce a git commit against the current tip, know who to pick as reviewers then rely on the people they pick to actually review the patch, then rebase and resubmit the patch because the tip has moved during the review process and then have someone with commit rights accept it. And the web interface is horrible. You may have guessed I'm not a fan ;-)
Hi Jon,
On 04/17/2013 06:29 AM, Jon Medhurst (Tixy) wrote:
On Wed, 2013-04-17 at 06:21 +0200, Jonathan Aquilina wrote:
Doesnt email run the risk of a patch slipping through the cracks?
And with gerrit the patch author needs to get an account enabled with the project, produce a git commit against the current tip,
I can't recall ever seeing an upload refused because it wasn't against the latest commit. What's the error message you get? Or is it possible that you might be misremembering such an incident?
know who to pick as reviewers then rely on the people they pick to actually review the patch
I fail to see how emailing patches approaches this problem any differently.
then rebase and resubmit the patch because the tip has moved during the review process
Perhaps it is a small fault of the software that the configuration doesn't default to an easier-going merge method such as cherry picking. It's really easy to change, though. Admin->Projects->NAME->Project Options->Cherry Pick. It seems like emailed patches have their fair share of merge conflicts.
and then have someone with commit rights accept it.
I fail to see how emailing patches approaches this problem any differently.
And the web interface is horrible.
What's your preferred intraline diff application?
You may have guessed I'm not a fan ;-)
I don't necessarily think Linaro needs more Gerrit, but I would hate to see misunderstandings about software that's been good to me go uncorrected.
Cheers, Christopher
On Wed, 2013-04-17 at 09:14 -0400, Christopher Covington wrote:
Hi Jon,
On 04/17/2013 06:29 AM, Jon Medhurst (Tixy) wrote:
On Wed, 2013-04-17 at 06:21 +0200, Jonathan Aquilina wrote:
Doesnt email run the risk of a patch slipping through the cracks?
And with gerrit the patch author needs to get an account enabled with the project, produce a git commit against the current tip,
I can't recall ever seeing an upload refused because it wasn't against the latest commit. What's the error message you get?
Can't remember, sorry.
Or is it possible that you might be misremembering such an incident?
know who to pick as reviewers then rely on the people they pick to actually review the patch
I fail to see how emailing patches approaches this problem any differently.
It doesn't.
then rebase and resubmit the patch because the tip has moved during the review process
Perhaps it is a small fault of the software that the configuration doesn't default to an easier-going merge method such as cherry picking. It's really easy to change, though. Admin->Projects->NAME->Project Options->Cherry Pick. It seems like emailed patches have their fair share of merge conflicts.
and then have someone with commit rights accept it.
I fail to see how emailing patches approaches this problem any differently.
It doesn't.
And the web interface is horrible.
What's your preferred intraline diff application?
Not just diff, also how you comment on changes and reply to them. Email is much nicer.
You may have guessed I'm not a fan ;-)
I don't necessarily think Linaro needs more Gerrit, but I would hate to see misunderstandings about software that's been good to me go uncorrected.
And I wasn't particularly trying to imply mailing lists solve problems that Gerrit fails to, just that Gerrit adds hurdles and awkwardness - especially for occasional contributors - and it seems to add little benefit for that extra pain. You could say it keeps track of changes and avoid them getting lost, but that only works if someone actually chases up neglected changes. And mailing lists can use something like Patchwork if they want similar tracking.
I'm sure if I worked regularly on a project which used Gerrit then I would get use to it and work out the tricks.
Cheers
I have seen gerrit in use with libreoffice, they have it setup to where you have build bot machines which can automatically pull a patch make a build and test the patch with the build to see if it builds successfully.
Also, with gerrit you dont need to assign reviewers people that have the ability and access can assign themselves to a particular patch to test.
On Wed, Apr 17, 2013 at 3:55 PM, Jon Medhurst (Tixy) tixy@linaro.orgwrote:
On Wed, 2013-04-17 at 09:14 -0400, Christopher Covington wrote:
Hi Jon,
On 04/17/2013 06:29 AM, Jon Medhurst (Tixy) wrote:
On Wed, 2013-04-17 at 06:21 +0200, Jonathan Aquilina wrote:
Doesnt email run the risk of a patch slipping through the cracks?
And with gerrit the patch author needs to get an account enabled with
the
project, produce a git commit against the current tip,
I can't recall ever seeing an upload refused because it wasn't against
the
latest commit. What's the error message you get?
Can't remember, sorry.
Or is it possible that you might be misremembering such an incident?
know who to pick as reviewers then rely on the people they pick to actually review the patch
I fail to see how emailing patches approaches this problem any
differently.
It doesn't.
then rebase and resubmit the patch because the tip has moved during the review process
Perhaps it is a small fault of the software that the configuration
doesn't
default to an easier-going merge method such as cherry picking. It's
really
easy to change, though. Admin->Projects->NAME->Project Options->Cherry
Pick.
It seems like emailed patches have their fair share of merge conflicts.
and then have someone with commit rights accept it.
I fail to see how emailing patches approaches this problem any
differently.
It doesn't.
And the web interface is horrible.
What's your preferred intraline diff application?
Not just diff, also how you comment on changes and reply to them. Email is much nicer.
You may have guessed I'm not a fan ;-)
I don't necessarily think Linaro needs more Gerrit, but I would hate to
see
misunderstandings about software that's been good to me go uncorrected.
And I wasn't particularly trying to imply mailing lists solve problems that Gerrit fails to, just that Gerrit adds hurdles and awkwardness - especially for occasional contributors - and it seems to add little benefit for that extra pain. You could say it keeps track of changes and avoid them getting lost, but that only works if someone actually chases up neglected changes. And mailing lists can use something like Patchwork if they want similar tracking.
I'm sure if I worked regularly on a project which used Gerrit then I would get use to it and work out the tricks.
Cheers
-- Tixy
Cheers, Christopher
Christopher Covington cov@codeaurora.org writes:
On 04/17/2013 06:29 AM, Jon Medhurst (Tixy) wrote:
On Wed, 2013-04-17 at 06:21 +0200, Jonathan Aquilina wrote:
Doesnt email run the risk of a patch slipping through the cracks?
And with gerrit the patch author needs to get an account enabled with the project, produce a git commit against the current tip,
I can't recall ever seeing an upload refused because it wasn't against the latest commit. What's the error message you get? Or is it possible that you might be misremembering such an incident?
It'll depend on how the project is configured. If a project allows merges, or is configured to rebase automatically, it won't matter. You'll not ever be able to merge if the project is set to fast-forward-only. It didn't used to reject these uploads, but I could see that easily being something that would be added.
David
On Wed, Apr 17, 2013 at 09:14:54AM -0400, Christopher Covington wrote:
On 04/17/2013 06:29 AM, Jon Medhurst (Tixy) wrote:
And with gerrit the patch author needs to get an account enabled with the project, produce a git commit against the current tip,
I can't recall ever seeing an upload refused because it wasn't against the latest commit. What's the error message you get? Or is it possible that you might be misremembering such an incident?
This is a configuration option within gerrit - you can set a repository or branch up to be fast forward only. This is frequently done for kernels as it's relatively easy for a change in one part of the code base to interact with changes in another part (especially board changes interacting with the drivers they instantiate) so the theory is that this will force all the verification to be done with exactly the tree that ends up in gerrit.
Obviously there's a tradeoff here with the rebases.
On 04/28/2013 06:18 AM, Mark Brown wrote:
On Wed, Apr 17, 2013 at 09:14:54AM -0400, Christopher Covington wrote:
On 04/17/2013 06:29 AM, Jon Medhurst (Tixy) wrote:
And with gerrit the patch author needs to get an account enabled with the project, produce a git commit against the current tip,
I can't recall ever seeing an upload refused because it wasn't against the latest commit. What's the error message you get? Or is it possible that you might be misremembering such an incident?
This is a configuration option within gerrit - you can set a repository or branch up to be fast forward only.
I'm familiar with a submit- or merge-time option for this. It's not clear to me from your reply whether you're referring to this or an upload- or push-time option.
Thanks, Christopher
On 04/29/2013 09:28 AM, Christopher Covington wrote:
On 04/28/2013 06:18 AM, Mark Brown wrote:
On Wed, Apr 17, 2013 at 09:14:54AM -0400, Christopher Covington wrote:
On 04/17/2013 06:29 AM, Jon Medhurst (Tixy) wrote:
And with gerrit the patch author needs to get an account enabled with the project, produce a git commit against the current tip,
I can't recall ever seeing an upload refused because it wasn't against the latest commit. What's the error message you get? Or is it possible that you might be misremembering such an incident?
This is a configuration option within gerrit - you can set a repository or branch up to be fast forward only.
I'm familiar with a submit- or merge-time option for this. It's not clear to me from your reply whether you're referring to this or an upload- or push-time option.
Ohhh. In talking about Gerrit I assumed a workflow that used code review, but in the case where review is bypassed and one pushes to refs/heads/$branch instead of refs/for/$branch, it makes total sense for the restrictions that are seen from the code review workflow when you hit the "submit" button to applied to pushes/uploads. Sorry for my confusion.
Christopher
On Mon, Apr 29, 2013 at 09:28:16AM -0400, Christopher Covington wrote:
I'm familiar with a submit- or merge-time option for this. It's not clear to me from your reply whether you're referring to this or an upload- or push-time option.
Oh, on initial push? I've not seen that but it does strike me that it would be somewhat useful in avoiding frustration from things not merging later on - at least you know there was a chance that the change could've been merged.
With gerrit have the ability to pull patch sets build and test. On Apr 29, 2013 10:04 PM, "Mark Brown" broonie@sirena.org.uk wrote:
On Mon, Apr 29, 2013 at 09:28:16AM -0400, Christopher Covington wrote:
I'm familiar with a submit- or merge-time option for this. It's not
clear to
me from your reply whether you're referring to this or an upload- or
push-time
option.
Oh, on initial push? I've not seen that but it does strike me that it would be somewhat useful in avoiding frustration from things not merging later on - at least you know there was a chance that the change could've been merged.
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
-
Christopher Covington -
David Brown -
Deepak Saxena -
Jon Medhurst (Tixy) -
Jonathan Aquilina -
Mark Brown -
Nicolas Dechesne