[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] We need more reviewers/maintainers!!
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] We need more reviewers/maintainers!! |
Date: |
Wed, 14 Mar 2012 20:00:13 +0000 |
On Tue, Mar 13, 2012 at 14:41, Anthony Liguori <address@hidden> wrote:
> On 03/13/2012 09:38 AM, Avi Kivity wrote:
>>
>> On 03/13/2012 04:00 PM, Anthony Liguori wrote:
>>>
>>> On 03/13/2012 08:40 AM, Avi Kivity wrote:
>>>>
>>>> On 03/12/2012 10:27 PM, Anthony Liguori wrote:
>>>>>>
>>>>>> I agree that more maintainers would be good, but we also need
>>>>>> more people with commit rights.
>>>>>
>>>>>
>>>>> I disagree strongly. Having multiple pushers makes things difficult
>>>>> and encourages people to push without testing. Part of what makes
>>>>> pushing take longer than it should today is that my test cycle takes
>>>>> at least 1-2 hours and it's not uncommon to have to go through 3-4
>>>>> cycles of rebasing before being able to push.
>>>>
>>>>
>>>> This really sucks.
>>>>
>>>> If testing was automated, we could have a staging branch where
>>>> maintainers would push patches, they'd get tested automatically and then
>>>> graduate to master. The workflow would look something like
>>>>
>>>> git fetch
>>>> git checkout staging
>>>> git rebase origin/staging
>>>> <apply patches, pull trees>
>>>> git push staging
>>>> <wait>
>>>> <staging gets merged into master autoamatically, or you get an email
>>>> from the test system>
>>>
>>>
>>> The problem for me with this is that I test before I do a thorough
>>> review. I do a quick review, but not a line-by-line review. So I
>>> don't necessarily want to queue for push.
>>
>>
>> Seems to me it's better to review before testing, no?
>
>
> I typically do a high level review before queuing for testing, but I don't
> do a line-by-line review for coding style or minor issues.
>
> The later must be done before committing no matter how many revisions are
> sent. It's more time efficient to catch a functional problem without doing
> the line-by-line review. Best case scenario is that the line-by-line review
> happens only once for a patch before it's committed.
Perhaps patchwork is not the right tool for this. Coreboot uses
Gerrit: http://review.coreboot.org/#/q/status:open,n,z combined with
Jenkins build bot. This looks much more professional.
>>>> If testing cannot be automated, perhaps a lock around the tree would
>>>> help.
>>>
>>>
>>> I think merging qemu-test into make check would help a lot. If all
>>> committers are running the same test suite before pushing, then this
>>> problem would become less common. It's livable now because most
>>> committers commit infrequently.
>>>
>>> But if we added more committers, it would become pretty problematic.
>>
>>
>> I'm not arguing either for or against that, just trying to make the
>> commit process more efficient.
>
>
> Yup, and appreciate the suggestions.
>
> Regards,
>
> Anthony Liguori
>
>
>
- Re: [Qemu-devel] We need more reviewers/maintainers!!, (continued)
- Re: [Qemu-devel] We need more reviewers/maintainers!!, Alexander Graf, 2012/03/13
- Re: [Qemu-devel] We need more reviewers/maintainers!!, Peter Maydell, 2012/03/13
- Re: [Qemu-devel] We need more reviewers/maintainers!!, Andreas Färber, 2012/03/13
- Re: [Qemu-devel] We need more reviewers/maintainers!!, Avi Kivity, 2012/03/13
- Re: [Qemu-devel] We need more reviewers/maintainers!!, Eric Blake, 2012/03/13
- Re: [Qemu-devel] We need more reviewers/maintainers!!, Stefan Weil, 2012/03/12
- Re: [Qemu-devel] We need more reviewers/maintainers!!, Avi Kivity, 2012/03/13
- Re: [Qemu-devel] We need more reviewers/maintainers!!, Anthony Liguori, 2012/03/13
- Re: [Qemu-devel] We need more reviewers/maintainers!!, Avi Kivity, 2012/03/13
- Re: [Qemu-devel] We need more reviewers/maintainers!!, Anthony Liguori, 2012/03/13
- Re: [Qemu-devel] We need more reviewers/maintainers!!,
Blue Swirl <=
- Re: [Qemu-devel] We need more reviewers/maintainers!!, Blue Swirl, 2012/03/14
Re: [Qemu-devel] We need more reviewers/maintainers!!, Stefan Hajnoczi, 2012/03/13