geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Xiaojian Zhou <gz...@pivotal.io>
Subject Re: PRs should always include tests
Date Fri, 29 Dec 2017 22:20:00 GMT
How about the code change is already covered by existing tests?

Not to reduce test coverage seems a more reasonable standard.

On Fri, Dec 29, 2017 at 2:07 PM, Udo Kohlmeyer <udo@apache.org> wrote:

> +1
>
>
>
> On 12/29/17 12:05, Kirk Lund wrote:
>
>> I think we all need to be very consistent in requiring tests with all PRs.
>> This goes for committer as well as non-committer contributions.
>>
>> A test would both confirm the existence of the bug in the first place and
>> then confirm the fix. Without such a test, any developer could come along
>> later, modify the code in question and break it without ever realizing it.
>> A test would protect the behavior that was fixed or introduced.
>>
>> Also if we are not consistent in requiring tests for all contributions,
>> then contributors will learn to pick and choose which reviewers to listen
>> to and which ones to ignore.
>>
>> I for one do not want to waste my time reviewing and requesting changes
>> only to be ignored and have said PR be committed without the (justified)
>> changes I've requested.
>>
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message