You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by John Kinsella <jl...@stratosec.co> on 2012/07/30 19:26:25 UTC
Please remember to close review board requests
Guys and gals - if you have a request in ReviewBoard that's been marked "Ship It!" - it might not be obvious, but you need to go back and close the request by marking it "Submitted."
Please remember to do this after momentarily basking in the joy of having your patch accepted. :)
That'll save us from having to spam the dev list with requests to close out tickets, and allow us to see easier what needs to be reviewed.
John
Re: Please remember to close review board requests
Posted by Alena Prokharchyk <Al...@citrix.com>.
On 7/30/12 1:28 PM, "Edison Su" <Ed...@citrix.com> wrote:
>
>
>> -----Original Message-----
>> From: Ewan Mellor [mailto:Ewan.Mellor@eu.citrix.com]
>> Sent: Monday, July 30, 2012 1:01 PM
>> To: cloudstack-dev@incubator.apache.org
>> Subject: RE: Please remember to close review board requests
>>
>> > -----Original Message-----
>> > From: John Kinsella [mailto:jlk@stratosec.co]
>> > Sent: Monday, July 30, 2012 10:26 AM
>> > To: cloudstack-dev@incubator.apache.org
>> > Subject: Please remember to close review board requests
>> >
>> > Guys and gals - if you have a request in ReviewBoard that's been
>> marked
>> > "Ship It!" - it might not be obvious, but you need to go back and
>> close
>> > the request by marking it "Submitted."
>> >
>> > Please remember to do this after momentarily basking in the joy of
>> > having your patch accepted. :)
>> >
>> > That'll save us from having to spam the dev list with requests to
>> close
>> > out tickets, and allow us to see easier what needs to be reviewed.
>>
>> Shouldn't the committer do this? If I understand correctly, the
>> workflow is:
>>
>> 1. patch gets approved.
>> 2. committer merges patch manually.
>>
>> If number 1 happens, but then someone gets distracted before they do
>> number 2, then we don't want the ticket being closed as submitted. We
>> should have the committer do it after they push (or even better, have
>> some automated system do it).
>
>I tried, but seems as a committer, you can't change the status to
>"Submitted" for a specific patch.
>
>>
>> Ewan.
>
>
+1. I could never submit the patches I've reviewed - there is no such
option provided.
RE: Please remember to close review board requests
Posted by Edison Su <Ed...@citrix.com>.
> -----Original Message-----
> From: Ewan Mellor [mailto:Ewan.Mellor@eu.citrix.com]
> Sent: Monday, July 30, 2012 1:01 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: RE: Please remember to close review board requests
>
> > -----Original Message-----
> > From: John Kinsella [mailto:jlk@stratosec.co]
> > Sent: Monday, July 30, 2012 10:26 AM
> > To: cloudstack-dev@incubator.apache.org
> > Subject: Please remember to close review board requests
> >
> > Guys and gals - if you have a request in ReviewBoard that's been
> marked
> > "Ship It!" - it might not be obvious, but you need to go back and
> close
> > the request by marking it "Submitted."
> >
> > Please remember to do this after momentarily basking in the joy of
> > having your patch accepted. :)
> >
> > That'll save us from having to spam the dev list with requests to
> close
> > out tickets, and allow us to see easier what needs to be reviewed.
>
> Shouldn't the committer do this? If I understand correctly, the
> workflow is:
>
> 1. patch gets approved.
> 2. committer merges patch manually.
>
> If number 1 happens, but then someone gets distracted before they do
> number 2, then we don't want the ticket being closed as submitted. We
> should have the committer do it after they push (or even better, have
> some automated system do it).
I tried, but seems as a committer, you can't change the status to "Submitted" for a specific patch.
>
> Ewan.
Re: Please remember to close review board requests
Posted by Prasanna Santhanam <Pr...@citrix.com>.
On Mon, Jul 30, 2012 at 06:55:04PM -0400, Ewan Mellor wrote:
> > -----Original Message-----
> > From: John Kinsella [mailto:jlk@stratosec.co]
> > Sent: Monday, July 30, 2012 1:30 PM
> > To: <cl...@incubator.apache.org>
> > Subject: Re: Please remember to close review board requests
> >
> > [Snip]
> >
> > The patch submitter has to mark the issue as closed - as a committer I
> > do not have the ability to close a review request. I've reviewed the
> > ReviewBoard docs[0] and this seems to be purposeful - I think the
> > intended use case is for authors who have commit access on the source
> > tree.
>
> We really have to figure out how to automate this. People are just
> going to make mistakes in the current setup.
>
> Does anyone want to volunteer to be Head Toolsmith? Guaranteed
> fame, fortune, and infinite gratitude awaits anyone willing to
> invest time in our tooling. The Cloud Project That Cannot Be Named
> has excellent tooling, and we should copy swathes of that work for
> ourselves. I would be glad to provide requirements, advice, and
> guidance to anyone with time to donate.
>
> Thanks,
>
> Ewan.
Since that project is mostly python code I can lend a helping hand
with any python scripts.
--
Prasanna.,
RE: Please remember to close review board requests
Posted by Ewan Mellor <Ew...@eu.citrix.com>.
> -----Original Message-----
> From: John Kinsella [mailto:jlk@stratosec.co]
> Sent: Monday, July 30, 2012 1:30 PM
> To: <cl...@incubator.apache.org>
> Subject: Re: Please remember to close review board requests
>
> [Snip]
>
> The patch submitter has to mark the issue as closed - as a committer I
> do not have the ability to close a review request. I've reviewed the
> ReviewBoard docs[0] and this seems to be purposeful - I think the
> intended use case is for authors who have commit access on the source
> tree.
We really have to figure out how to automate this. People are just going to make mistakes in the current setup.
Does anyone want to volunteer to be Head Toolsmith? Guaranteed fame, fortune, and infinite gratitude awaits anyone willing to invest time in our tooling. The Cloud Project That Cannot Be Named has excellent tooling, and we should copy swathes of that work for ourselves. I would be glad to provide requirements, advice, and guidance to anyone with time to donate.
Thanks,
Ewan.
Re: Please remember to close review board requests
Posted by John Kinsella <jl...@stratosec.co>.
On Jul 30, 2012, at 1:01 PM, Ewan Mellor wrote:
>> -----Original Message-----
>> From: John Kinsella [mailto:jlk@stratosec.co]
>> Sent: Monday, July 30, 2012 10:26 AM
>> To: cloudstack-dev@incubator.apache.org
>> Subject: Please remember to close review board requests
>>
>> Guys and gals - if you have a request in ReviewBoard that's been marked
>> "Ship It!" - it might not be obvious, but you need to go back and close
>> the request by marking it "Submitted."
>>
>> Please remember to do this after momentarily basking in the joy of
>> having your patch accepted. :)
>>
>> That'll save us from having to spam the dev list with requests to close
>> out tickets, and allow us to see easier what needs to be reviewed.
>
> Shouldn't the committer do this? If I understand correctly, the workflow is:
>
> 1. patch gets approved.
> 2. committer merges patch manually.
>
> If number 1 happens, but then someone gets distracted before they do number 2, then we don't want the ticket being closed as submitted. We should have the committer do it after they push (or even better, have some automated system do it).
It's the committer's responsibility to review the patch and provide feedback or accept the patch and commit it, if the patch submitter does not have commit permissions.
The patch submitter has to mark the issue as closed - as a committer I do not have the ability to close a review request. I've reviewed the ReviewBoard docs[0] and this seems to be purposeful - I think the intended use case is for authors who have commit access on the source tree.
John
0:http://www.reviewboard.org/docs/manual/1.6/users/getting-started/workflow/
RE: Please remember to close review board requests
Posted by Ewan Mellor <Ew...@eu.citrix.com>.
> -----Original Message-----
> From: John Kinsella [mailto:jlk@stratosec.co]
> Sent: Monday, July 30, 2012 10:26 AM
> To: cloudstack-dev@incubator.apache.org
> Subject: Please remember to close review board requests
>
> Guys and gals - if you have a request in ReviewBoard that's been marked
> "Ship It!" - it might not be obvious, but you need to go back and close
> the request by marking it "Submitted."
>
> Please remember to do this after momentarily basking in the joy of
> having your patch accepted. :)
>
> That'll save us from having to spam the dev list with requests to close
> out tickets, and allow us to see easier what needs to be reviewed.
Shouldn't the committer do this? If I understand correctly, the workflow is:
1. patch gets approved.
2. committer merges patch manually.
If number 1 happens, but then someone gets distracted before they do number 2, then we don't want the ticket being closed as submitted. We should have the committer do it after they push (or even better, have some automated system do it).
Ewan.