You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@aurora.apache.org by Joshua Cohen <jc...@twopensource.com> on 2015/02/23 19:35:48 UTC
Testing Done in RB?
This came up in a review and I figured it'd be better discussed here rather
than in a review that most folks probably aren't reading.
Do people find value in this section being filled in? The argument against
is generally that it's always the same value and if we wait for a ship it
from ReviewBot we have assurance that everything is kosher. The argument
for is that it serves as an example for new committers on how to run tests,
however this was on the assumption that the full commit message from `rbt
patch` was used. It seems that some people edit the testing done section
out before merging commits from RB(?).
Thoughts?
Cheers,
Joshua
Re: Testing Done in RB?
Posted by Joshua Cohen <jc...@twopensource.com>.
Thanks Joe!
On Tue, Feb 24, 2015 at 11:22 PM, Joseph Smith <ya...@gmail.com> wrote:
> I’m ambivalent either way, but overall sounds like a good path forward, so
> filed https://issues.apache.org/jira/browse/AURORA-1147 <
> https://issues.apache.org/jira/browse/AURORA-1147>
>
>
> > On Feb 23, 2015, at 10:55, Joshua Cohen <jc...@twopensource.com> wrote:
> >
> > If we decide to remove the 'Testing Done' section from commit messages
> then
> > we should definitely update CONTRIBUTING.md[1] to codify the practice.
> >
> > [1]
> https://github.com/apache/incubator-aurora/blob/master/CONTRIBUTING.md
> >
> > On Mon, Feb 23, 2015 at 10:52 AM, Bill Farner <wf...@apache.org>
> wrote:
> >
> >> I'm generally -1 to requiring humans to perform (what i perceive to be)
> >> superfluous redundant tasks.
> >>
> >> In this case, the standard suite of tests should be expected as part of
> any
> >> patch affecting code, which is why the review bot runs them. For that
> >> reason, i've mostly been using 'Testing done' to reference additional
> >> testing.
> >>
> >> I'm a stronger -1 on including this field in commit messages, for the
> >> reason Maxim notes above.
> >>
> >> -=Bill
> >>
> >> On Mon, Feb 23, 2015 at 10:45 AM, Maxim Khutornenko <ma...@apache.org>
> >> wrote:
> >>
> >>> I am +1 on filling the test section as it may be a good indicator of
> >>> what testing has been attempted (e.g. java-only, python-only, python
> >>> sub-target, vagrant e2e, vagrant manual).
> >>>
> >>> I am -1 on having this included into the commit message as it blows up
> >>> the commit message size and makes for a hard to read arbitrary
> >>> length/formatted commit. The RB link should be enough to get commit
> >>> details and testing done.
> >>>
> >>> On Mon, Feb 23, 2015 at 10:35 AM, Joshua Cohen <
> jcohen@twopensource.com>
> >>> wrote:
> >>>> This came up in a review and I figured it'd be better discussed here
> >>> rather
> >>>> than in a review that most folks probably aren't reading.
> >>>>
> >>>> Do people find value in this section being filled in? The argument
> >>> against
> >>>> is generally that it's always the same value and if we wait for a ship
> >> it
> >>>> from ReviewBot we have assurance that everything is kosher. The
> >> argument
> >>>> for is that it serves as an example for new committers on how to run
> >>> tests,
> >>>> however this was on the assumption that the full commit message from
> >> `rbt
> >>>> patch` was used. It seems that some people edit the testing done
> >> section
> >>>> out before merging commits from RB(?).
> >>>>
> >>>> Thoughts?
> >>>>
> >>>> Cheers,
> >>>>
> >>>> Joshua
> >>>
> >>
>
>
Re: Testing Done in RB?
Posted by Joseph Smith <ya...@gmail.com>.
I’m ambivalent either way, but overall sounds like a good path forward, so filed https://issues.apache.org/jira/browse/AURORA-1147 <https://issues.apache.org/jira/browse/AURORA-1147>
> On Feb 23, 2015, at 10:55, Joshua Cohen <jc...@twopensource.com> wrote:
>
> If we decide to remove the 'Testing Done' section from commit messages then
> we should definitely update CONTRIBUTING.md[1] to codify the practice.
>
> [1] https://github.com/apache/incubator-aurora/blob/master/CONTRIBUTING.md
>
> On Mon, Feb 23, 2015 at 10:52 AM, Bill Farner <wf...@apache.org> wrote:
>
>> I'm generally -1 to requiring humans to perform (what i perceive to be)
>> superfluous redundant tasks.
>>
>> In this case, the standard suite of tests should be expected as part of any
>> patch affecting code, which is why the review bot runs them. For that
>> reason, i've mostly been using 'Testing done' to reference additional
>> testing.
>>
>> I'm a stronger -1 on including this field in commit messages, for the
>> reason Maxim notes above.
>>
>> -=Bill
>>
>> On Mon, Feb 23, 2015 at 10:45 AM, Maxim Khutornenko <ma...@apache.org>
>> wrote:
>>
>>> I am +1 on filling the test section as it may be a good indicator of
>>> what testing has been attempted (e.g. java-only, python-only, python
>>> sub-target, vagrant e2e, vagrant manual).
>>>
>>> I am -1 on having this included into the commit message as it blows up
>>> the commit message size and makes for a hard to read arbitrary
>>> length/formatted commit. The RB link should be enough to get commit
>>> details and testing done.
>>>
>>> On Mon, Feb 23, 2015 at 10:35 AM, Joshua Cohen <jc...@twopensource.com>
>>> wrote:
>>>> This came up in a review and I figured it'd be better discussed here
>>> rather
>>>> than in a review that most folks probably aren't reading.
>>>>
>>>> Do people find value in this section being filled in? The argument
>>> against
>>>> is generally that it's always the same value and if we wait for a ship
>> it
>>>> from ReviewBot we have assurance that everything is kosher. The
>> argument
>>>> for is that it serves as an example for new committers on how to run
>>> tests,
>>>> however this was on the assumption that the full commit message from
>> `rbt
>>>> patch` was used. It seems that some people edit the testing done
>> section
>>>> out before merging commits from RB(?).
>>>>
>>>> Thoughts?
>>>>
>>>> Cheers,
>>>>
>>>> Joshua
>>>
>>
Re: Testing Done in RB?
Posted by Joshua Cohen <jc...@twopensource.com>.
If we decide to remove the 'Testing Done' section from commit messages then
we should definitely update CONTRIBUTING.md[1] to codify the practice.
[1] https://github.com/apache/incubator-aurora/blob/master/CONTRIBUTING.md
On Mon, Feb 23, 2015 at 10:52 AM, Bill Farner <wf...@apache.org> wrote:
> I'm generally -1 to requiring humans to perform (what i perceive to be)
> superfluous redundant tasks.
>
> In this case, the standard suite of tests should be expected as part of any
> patch affecting code, which is why the review bot runs them. For that
> reason, i've mostly been using 'Testing done' to reference additional
> testing.
>
> I'm a stronger -1 on including this field in commit messages, for the
> reason Maxim notes above.
>
> -=Bill
>
> On Mon, Feb 23, 2015 at 10:45 AM, Maxim Khutornenko <ma...@apache.org>
> wrote:
>
> > I am +1 on filling the test section as it may be a good indicator of
> > what testing has been attempted (e.g. java-only, python-only, python
> > sub-target, vagrant e2e, vagrant manual).
> >
> > I am -1 on having this included into the commit message as it blows up
> > the commit message size and makes for a hard to read arbitrary
> > length/formatted commit. The RB link should be enough to get commit
> > details and testing done.
> >
> > On Mon, Feb 23, 2015 at 10:35 AM, Joshua Cohen <jc...@twopensource.com>
> > wrote:
> > > This came up in a review and I figured it'd be better discussed here
> > rather
> > > than in a review that most folks probably aren't reading.
> > >
> > > Do people find value in this section being filled in? The argument
> > against
> > > is generally that it's always the same value and if we wait for a ship
> it
> > > from ReviewBot we have assurance that everything is kosher. The
> argument
> > > for is that it serves as an example for new committers on how to run
> > tests,
> > > however this was on the assumption that the full commit message from
> `rbt
> > > patch` was used. It seems that some people edit the testing done
> section
> > > out before merging commits from RB(?).
> > >
> > > Thoughts?
> > >
> > > Cheers,
> > >
> > > Joshua
> >
>
Re: Testing Done in RB?
Posted by Bill Farner <wf...@apache.org>.
I'm generally -1 to requiring humans to perform (what i perceive to be)
superfluous redundant tasks.
In this case, the standard suite of tests should be expected as part of any
patch affecting code, which is why the review bot runs them. For that
reason, i've mostly been using 'Testing done' to reference additional
testing.
I'm a stronger -1 on including this field in commit messages, for the
reason Maxim notes above.
-=Bill
On Mon, Feb 23, 2015 at 10:45 AM, Maxim Khutornenko <ma...@apache.org>
wrote:
> I am +1 on filling the test section as it may be a good indicator of
> what testing has been attempted (e.g. java-only, python-only, python
> sub-target, vagrant e2e, vagrant manual).
>
> I am -1 on having this included into the commit message as it blows up
> the commit message size and makes for a hard to read arbitrary
> length/formatted commit. The RB link should be enough to get commit
> details and testing done.
>
> On Mon, Feb 23, 2015 at 10:35 AM, Joshua Cohen <jc...@twopensource.com>
> wrote:
> > This came up in a review and I figured it'd be better discussed here
> rather
> > than in a review that most folks probably aren't reading.
> >
> > Do people find value in this section being filled in? The argument
> against
> > is generally that it's always the same value and if we wait for a ship it
> > from ReviewBot we have assurance that everything is kosher. The argument
> > for is that it serves as an example for new committers on how to run
> tests,
> > however this was on the assumption that the full commit message from `rbt
> > patch` was used. It seems that some people edit the testing done section
> > out before merging commits from RB(?).
> >
> > Thoughts?
> >
> > Cheers,
> >
> > Joshua
>
Re: Testing Done in RB?
Posted by Maxim Khutornenko <ma...@apache.org>.
I am +1 on filling the test section as it may be a good indicator of
what testing has been attempted (e.g. java-only, python-only, python
sub-target, vagrant e2e, vagrant manual).
I am -1 on having this included into the commit message as it blows up
the commit message size and makes for a hard to read arbitrary
length/formatted commit. The RB link should be enough to get commit
details and testing done.
On Mon, Feb 23, 2015 at 10:35 AM, Joshua Cohen <jc...@twopensource.com> wrote:
> This came up in a review and I figured it'd be better discussed here rather
> than in a review that most folks probably aren't reading.
>
> Do people find value in this section being filled in? The argument against
> is generally that it's always the same value and if we wait for a ship it
> from ReviewBot we have assurance that everything is kosher. The argument
> for is that it serves as an example for new committers on how to run tests,
> however this was on the assumption that the full commit message from `rbt
> patch` was used. It seems that some people edit the testing done section
> out before merging commits from RB(?).
>
> Thoughts?
>
> Cheers,
>
> Joshua