You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by "Franklin, Matthew B." <mf...@mitre.org> on 2011/08/12 00:38:44 UTC
Reviewing patches
A recently submitted patch brought up a few questions that would be nice to ask in context of the code. It seems to me that we should have a structured way of reviewing and commenting on patches submitted by the community before application. I know some people have used http://codereview.appspot.com, but does anyone else have experience with public code review tools in open source?
-Matt
Re: Reviewing patches
Posted by Raminderjeet Singh <ra...@gmail.com>.
The base dir can be https://svn.apache.org/repos/asf/incubator/rave/trunk or https://svn.apache.org/repos/asf/incubator/rave/trunk/rave-portal etc based on from where the patch was created.
On Aug 16, 2011, at 2:19 PM, Jasha Joachimsthal wrote:
> What is the correct base dir? This is also something we should mention on
> the Rave website.
>
> On 16 August 2011 20:04, Raminderjeet Singh <ra...@gmail.com>wrote:
>
>> Jasha,
>>
>> I had the same problem if i selected wrong Base Dir. Hope that is the same
>> problem you faced. I am using this to review patch for RAVE-148.
>>
>> Thank
>> Raminder
>>
>> On Aug 16, 2011, at 4:23 AM, Jasha Joachimsthal wrote:
>>
>>> On 15 August 2011 15:05, Ate Douma <at...@douma.nu> wrote:
>>>
>>>> On 08/12/2011 04:35 PM, Ate Douma wrote:
>>>>
>>>>> On 08/12/2011 01:07 PM, Ciancetta, Jesse E. wrote:
>>>>>
>>>>>> A recently submitted patch brought up a few questions that would be
>> nice
>>>>>>>>>
>>>>>>>> to ask in context of the code. It seems to me that we should have a
>>>>>>> structured way of reviewing and commenting on patches submitted by
>> the
>>>>>>> community before application. I know some people have used
>>>>>>> http://codereview.appspot.com, but does anyone else have experience
>>>>>>> with
>>>>>>> public code review tools in open source?\
>>>>>>> +1
>>>>>>>
>>>>>>>
>>>>>>>> How about Crucible - http://www.atlassian.com/**software/crucible/<
>> http://www.atlassian.com/software/crucible/>,
>>>>>>>> I am
>>>>>>>>
>>>>>>> not sure if ASF only maintains a JIRA or if Crucible is also
>> available?
>>>>>>>
>>>>>>>>
>>>>>>>> Apache has a reviewboard instance running at:
>>>>>>> https://reviews.apache.org.
>>>>>>> See also [1]
>>>>>>> Many projects ASF projects already are using it, including for
>> instance
>>>>>>> Shindig
>>>>>>> [2].
>>>>>>> We only need to request our project to be added at infra by creating
>> an
>>>>>>> issue
>>>>>>> for it.
>>>>>>>
>>>>>>> If nobody objects, I could do so later today.
>>>>>>>
>>>>>>
>>>>>> +1
>>>>>>
>>>>>> I've used reviewboard to submit patches to Shindig -- works well and
>> is
>>>>>> easy
>>>>>> to use.
>>>>>>
>>>>>
>>>>> Thanks, good to hear (I haven't reviewboard myself yet).
>>>>>
>>>>> I've requested to add Rave to reviewboard:
>>>>>
>>>>> https://issues.apache.org/**jira/browse/INFRA-3846<
>> https://issues.apache.org/jira/browse/INFRA-3846>
>>>>>
>>>>
>>>> Rave group is now added to and available on reviewboard (see above
>> issue)
>>>
>>>
>>> I tried to upload a patch as new review request but I got the error that
>> a
>>> file doesn't exist in the repository (which is true because it is new).
>> :(
>>>
>>> Jasha
>>
>>
Re: Reviewing patches
Posted by Raminderjeet Singh <rs...@gmail.com>.
I have question for Applying/Reviewing the patch process. When a patch is create svn revision was different than when a reviewer is applying the patch. There were intermediate update to same files. While applying the patch reviewer see patch problems related to few files in the patch and generating .rej files. What should be process for this condition? Do i reject the patch and the patch generator need to create a new patch? or the reviewer need to merge the changes carefully and commit the code. Case may be patch is submitted by a external developer which is not directly involved with RAVE.
I am facing .rej issue for patch for Rave 148.
Thanks
Raminder
On Aug 16, 2011, at 2:19 PM, Jasha Joachimsthal wrote:
> What is the correct base dir? This is also something we should mention on
> the Rave website.
>
> On 16 August 2011 20:04, Raminderjeet Singh <ra...@gmail.com>wrote:
>
>> Jasha,
>>
>> I had the same problem if i selected wrong Base Dir. Hope that is the same
>> problem you faced. I am using this to review patch for RAVE-148.
>>
>> Thank
>> Raminder
>>
>> On Aug 16, 2011, at 4:23 AM, Jasha Joachimsthal wrote:
>>
>>> On 15 August 2011 15:05, Ate Douma <at...@douma.nu> wrote:
>>>
>>>> On 08/12/2011 04:35 PM, Ate Douma wrote:
>>>>
>>>>> On 08/12/2011 01:07 PM, Ciancetta, Jesse E. wrote:
>>>>>
>>>>>> A recently submitted patch brought up a few questions that would be
>> nice
>>>>>>>>>
>>>>>>>> to ask in context of the code. It seems to me that we should have a
>>>>>>> structured way of reviewing and commenting on patches submitted by
>> the
>>>>>>> community before application. I know some people have used
>>>>>>> http://codereview.appspot.com, but does anyone else have experience
>>>>>>> with
>>>>>>> public code review tools in open source?\
>>>>>>> +1
>>>>>>>
>>>>>>>
>>>>>>>> How about Crucible - http://www.atlassian.com/**software/crucible/<
>> http://www.atlassian.com/software/crucible/>,
>>>>>>>> I am
>>>>>>>>
>>>>>>> not sure if ASF only maintains a JIRA or if Crucible is also
>> available?
>>>>>>>
>>>>>>>>
>>>>>>>> Apache has a reviewboard instance running at:
>>>>>>> https://reviews.apache.org.
>>>>>>> See also [1]
>>>>>>> Many projects ASF projects already are using it, including for
>> instance
>>>>>>> Shindig
>>>>>>> [2].
>>>>>>> We only need to request our project to be added at infra by creating
>> an
>>>>>>> issue
>>>>>>> for it.
>>>>>>>
>>>>>>> If nobody objects, I could do so later today.
>>>>>>>
>>>>>>
>>>>>> +1
>>>>>>
>>>>>> I've used reviewboard to submit patches to Shindig -- works well and
>> is
>>>>>> easy
>>>>>> to use.
>>>>>>
>>>>>
>>>>> Thanks, good to hear (I haven't reviewboard myself yet).
>>>>>
>>>>> I've requested to add Rave to reviewboard:
>>>>>
>>>>> https://issues.apache.org/**jira/browse/INFRA-3846<
>> https://issues.apache.org/jira/browse/INFRA-3846>
>>>>>
>>>>
>>>> Rave group is now added to and available on reviewboard (see above
>> issue)
>>>
>>>
>>> I tried to upload a patch as new review request but I got the error that
>> a
>>> file doesn't exist in the repository (which is true because it is new).
>> :(
>>>
>>> Jasha
>>
>>
Re: Reviewing patches
Posted by Jasha Joachimsthal <j....@onehippo.com>.
What is the correct base dir? This is also something we should mention on
the Rave website.
On 16 August 2011 20:04, Raminderjeet Singh <ra...@gmail.com>wrote:
> Jasha,
>
> I had the same problem if i selected wrong Base Dir. Hope that is the same
> problem you faced. I am using this to review patch for RAVE-148.
>
> Thank
> Raminder
>
> On Aug 16, 2011, at 4:23 AM, Jasha Joachimsthal wrote:
>
> > On 15 August 2011 15:05, Ate Douma <at...@douma.nu> wrote:
> >
> >> On 08/12/2011 04:35 PM, Ate Douma wrote:
> >>
> >>> On 08/12/2011 01:07 PM, Ciancetta, Jesse E. wrote:
> >>>
> >>>> A recently submitted patch brought up a few questions that would be
> nice
> >>>>>>>
> >>>>>> to ask in context of the code. It seems to me that we should have a
> >>>>> structured way of reviewing and commenting on patches submitted by
> the
> >>>>> community before application. I know some people have used
> >>>>> http://codereview.appspot.com, but does anyone else have experience
> >>>>> with
> >>>>> public code review tools in open source?\
> >>>>> +1
> >>>>>
> >>>>>
> >>>>>> How about Crucible - http://www.atlassian.com/**software/crucible/<
> http://www.atlassian.com/software/crucible/>,
> >>>>>> I am
> >>>>>>
> >>>>> not sure if ASF only maintains a JIRA or if Crucible is also
> available?
> >>>>>
> >>>>>>
> >>>>>> Apache has a reviewboard instance running at:
> >>>>> https://reviews.apache.org.
> >>>>> See also [1]
> >>>>> Many projects ASF projects already are using it, including for
> instance
> >>>>> Shindig
> >>>>> [2].
> >>>>> We only need to request our project to be added at infra by creating
> an
> >>>>> issue
> >>>>> for it.
> >>>>>
> >>>>> If nobody objects, I could do so later today.
> >>>>>
> >>>>
> >>>> +1
> >>>>
> >>>> I've used reviewboard to submit patches to Shindig -- works well and
> is
> >>>> easy
> >>>> to use.
> >>>>
> >>>
> >>> Thanks, good to hear (I haven't reviewboard myself yet).
> >>>
> >>> I've requested to add Rave to reviewboard:
> >>>
> >>> https://issues.apache.org/**jira/browse/INFRA-3846<
> https://issues.apache.org/jira/browse/INFRA-3846>
> >>>
> >>
> >> Rave group is now added to and available on reviewboard (see above
> issue)
> >
> >
> > I tried to upload a patch as new review request but I got the error that
> a
> > file doesn't exist in the repository (which is true because it is new).
> :(
> >
> > Jasha
>
>
Re: Reviewing patches
Posted by Raminderjeet Singh <ra...@gmail.com>.
Jasha,
I had the same problem if i selected wrong Base Dir. Hope that is the same problem you faced. I am using this to review patch for RAVE-148.
Thank
Raminder
On Aug 16, 2011, at 4:23 AM, Jasha Joachimsthal wrote:
> On 15 August 2011 15:05, Ate Douma <at...@douma.nu> wrote:
>
>> On 08/12/2011 04:35 PM, Ate Douma wrote:
>>
>>> On 08/12/2011 01:07 PM, Ciancetta, Jesse E. wrote:
>>>
>>>> A recently submitted patch brought up a few questions that would be nice
>>>>>>>
>>>>>> to ask in context of the code. It seems to me that we should have a
>>>>> structured way of reviewing and commenting on patches submitted by the
>>>>> community before application. I know some people have used
>>>>> http://codereview.appspot.com, but does anyone else have experience
>>>>> with
>>>>> public code review tools in open source?\
>>>>> +1
>>>>>
>>>>>
>>>>>> How about Crucible - http://www.atlassian.com/**software/crucible/<http://www.atlassian.com/software/crucible/>,
>>>>>> I am
>>>>>>
>>>>> not sure if ASF only maintains a JIRA or if Crucible is also available?
>>>>>
>>>>>>
>>>>>> Apache has a reviewboard instance running at:
>>>>> https://reviews.apache.org.
>>>>> See also [1]
>>>>> Many projects ASF projects already are using it, including for instance
>>>>> Shindig
>>>>> [2].
>>>>> We only need to request our project to be added at infra by creating an
>>>>> issue
>>>>> for it.
>>>>>
>>>>> If nobody objects, I could do so later today.
>>>>>
>>>>
>>>> +1
>>>>
>>>> I've used reviewboard to submit patches to Shindig -- works well and is
>>>> easy
>>>> to use.
>>>>
>>>
>>> Thanks, good to hear (I haven't reviewboard myself yet).
>>>
>>> I've requested to add Rave to reviewboard:
>>>
>>> https://issues.apache.org/**jira/browse/INFRA-3846<https://issues.apache.org/jira/browse/INFRA-3846>
>>>
>>
>> Rave group is now added to and available on reviewboard (see above issue)
>
>
> I tried to upload a patch as new review request but I got the error that a
> file doesn't exist in the repository (which is true because it is new). :(
>
> Jasha
Re: Reviewing patches
Posted by Jasha Joachimsthal <j....@onehippo.com>.
On 15 August 2011 15:05, Ate Douma <at...@douma.nu> wrote:
> On 08/12/2011 04:35 PM, Ate Douma wrote:
>
>> On 08/12/2011 01:07 PM, Ciancetta, Jesse E. wrote:
>>
>>> A recently submitted patch brought up a few questions that would be nice
>>>>>>
>>>>> to ask in context of the code. It seems to me that we should have a
>>>> structured way of reviewing and commenting on patches submitted by the
>>>> community before application. I know some people have used
>>>> http://codereview.appspot.com, but does anyone else have experience
>>>> with
>>>> public code review tools in open source?\
>>>> +1
>>>>
>>>>
>>>>> How about Crucible - http://www.atlassian.com/**software/crucible/<http://www.atlassian.com/software/crucible/>,
>>>>> I am
>>>>>
>>>> not sure if ASF only maintains a JIRA or if Crucible is also available?
>>>>
>>>>>
>>>>> Apache has a reviewboard instance running at:
>>>> https://reviews.apache.org.
>>>> See also [1]
>>>> Many projects ASF projects already are using it, including for instance
>>>> Shindig
>>>> [2].
>>>> We only need to request our project to be added at infra by creating an
>>>> issue
>>>> for it.
>>>>
>>>> If nobody objects, I could do so later today.
>>>>
>>>
>>> +1
>>>
>>> I've used reviewboard to submit patches to Shindig -- works well and is
>>> easy
>>> to use.
>>>
>>
>> Thanks, good to hear (I haven't reviewboard myself yet).
>>
>> I've requested to add Rave to reviewboard:
>>
>> https://issues.apache.org/**jira/browse/INFRA-3846<https://issues.apache.org/jira/browse/INFRA-3846>
>>
>
> Rave group is now added to and available on reviewboard (see above issue)
I tried to upload a patch as new review request but I got the error that a
file doesn't exist in the repository (which is true because it is new). :(
Jasha
Re: Reviewing patches
Posted by Ate Douma <at...@douma.nu>.
On 08/12/2011 04:35 PM, Ate Douma wrote:
> On 08/12/2011 01:07 PM, Ciancetta, Jesse E. wrote:
>>>>> A recently submitted patch brought up a few questions that would be nice
>>> to ask in context of the code. It seems to me that we should have a
>>> structured way of reviewing and commenting on patches submitted by the
>>> community before application. I know some people have used
>>> http://codereview.appspot.com, but does anyone else have experience with
>>> public code review tools in open source?\
>>> +1
>>>
>>>>
>>>> How about Crucible - http://www.atlassian.com/software/crucible/, I am
>>> not sure if ASF only maintains a JIRA or if Crucible is also available?
>>>>
>>> Apache has a reviewboard instance running at: https://reviews.apache.org.
>>> See also [1]
>>> Many projects ASF projects already are using it, including for instance Shindig
>>> [2].
>>> We only need to request our project to be added at infra by creating an issue
>>> for it.
>>>
>>> If nobody objects, I could do so later today.
>>
>> +1
>>
>> I've used reviewboard to submit patches to Shindig -- works well and is easy
>> to use.
>
> Thanks, good to hear (I haven't reviewboard myself yet).
>
> I've requested to add Rave to reviewboard:
>
> https://issues.apache.org/jira/browse/INFRA-3846
Rave group is now added to and available on reviewboard (see above issue)
>
> Ate
>
>>
>>> Ate
>>>
>>> [1]
>>> https://blogs.apache.org/infra/entry/reviewboard_instance_running_at_the
>>> [2] https://issues.apache.org/jira/browse/INFRA-3097
>>>
>>>> Suresh
>>>>
>>>>>
>>>>> -Matt
>>>>
>>
>
Re: Reviewing patches
Posted by Ate Douma <at...@douma.nu>.
On 08/12/2011 01:07 PM, Ciancetta, Jesse E. wrote:
>>>> A recently submitted patch brought up a few questions that would be nice
>> to ask in context of the code. It seems to me that we should have a
>> structured way of reviewing and commenting on patches submitted by the
>> community before application. I know some people have used
>> http://codereview.appspot.com, but does anyone else have experience with
>> public code review tools in open source?\
>> +1
>>
>>>
>>> How about Crucible - http://www.atlassian.com/software/crucible/, I am
>> not sure if ASF only maintains a JIRA or if Crucible is also available?
>>>
>> Apache has a reviewboard instance running at: https://reviews.apache.org.
>> See also [1]
>> Many projects ASF projects already are using it, including for instance Shindig
>> [2].
>> We only need to request our project to be added at infra by creating an issue
>> for it.
>>
>> If nobody objects, I could do so later today.
>
> +1
>
> I've used reviewboard to submit patches to Shindig -- works well and is easy to use.
Thanks, good to hear (I haven't reviewboard myself yet).
I've requested to add Rave to reviewboard:
https://issues.apache.org/jira/browse/INFRA-3846
Ate
>
>> Ate
>>
>> [1]
>> https://blogs.apache.org/infra/entry/reviewboard_instance_running_at_the
>> [2] https://issues.apache.org/jira/browse/INFRA-3097
>>
>>> Suresh
>>>
>>>>
>>>> -Matt
>>>
>
RE: Reviewing patches
Posted by "Ciancetta, Jesse E." <jc...@mitre.org>.
>>> A recently submitted patch brought up a few questions that would be nice
>to ask in context of the code. It seems to me that we should have a
>structured way of reviewing and commenting on patches submitted by the
>community before application. I know some people have used
>http://codereview.appspot.com, but does anyone else have experience with
>public code review tools in open source?\
>+1
>
>>
>> How about Crucible - http://www.atlassian.com/software/crucible/, I am
>not sure if ASF only maintains a JIRA or if Crucible is also available?
>>
>Apache has a reviewboard instance running at: https://reviews.apache.org.
>See also [1]
>Many projects ASF projects already are using it, including for instance Shindig
>[2].
>We only need to request our project to be added at infra by creating an issue
>for it.
>
>If nobody objects, I could do so later today.
+1
I've used reviewboard to submit patches to Shindig -- works well and is easy to use.
>Ate
>
>[1]
>https://blogs.apache.org/infra/entry/reviewboard_instance_running_at_the
>[2] https://issues.apache.org/jira/browse/INFRA-3097
>
>> Suresh
>>
>>>
>>> -Matt
>>
Re: Reviewing patches
Posted by Ate Douma <at...@douma.nu>.
On 08/12/2011 02:22 AM, Suresh Marru wrote:
>
> On Aug 11, 2011, at 6:38 PM, Franklin, Matthew B. wrote:
>
>> A recently submitted patch brought up a few questions that would be nice to ask in context of the code. It seems to me that we should have a structured way of reviewing and commenting on patches submitted by the community before application. I know some people have used http://codereview.appspot.com, but does anyone else have experience with public code review tools in open source?\
+1
>
> How about Crucible - http://www.atlassian.com/software/crucible/, I am not sure if ASF only maintains a JIRA or if Crucible is also available?
>
Apache has a reviewboard instance running at: https://reviews.apache.org.
See also [1]
Many projects ASF projects already are using it, including for instance Shindig [2].
We only need to request our project to be added at infra by creating an issue
for it.
If nobody objects, I could do so later today.
Ate
[1] https://blogs.apache.org/infra/entry/reviewboard_instance_running_at_the
[2] https://issues.apache.org/jira/browse/INFRA-3097
> Suresh
>
>>
>> -Matt
>
Re: Reviewing patches
Posted by Suresh Marru <sm...@apache.org>.
On Aug 11, 2011, at 6:38 PM, Franklin, Matthew B. wrote:
> A recently submitted patch brought up a few questions that would be nice to ask in context of the code. It seems to me that we should have a structured way of reviewing and commenting on patches submitted by the community before application. I know some people have used http://codereview.appspot.com, but does anyone else have experience with public code review tools in open source?\
How about Crucible - http://www.atlassian.com/software/crucible/, I am not sure if ASF only maintains a JIRA or if Crucible is also available?
Suresh
>
> -Matt