[Freeipa-devel] patch acceptance criteria

[Rob Crittenden]

Martin Kosek wrote:
> On 12/03/2015 09:08 AM, Petr Spacek wrote:
>> On 2.12.2015 19:14, Rob Crittenden wrote:
>>> Is it still mandatory that tests pass the unit tests before acceptance?
>>> I've seen a number of cases over the past couple of months where a
>>> change goes through then shortly afterward a patch to fix the tests.
>>> IMHO this should be caught in advance.
>>>
>>> Things slip through and goodness knows I've acked more than a few
>>> patches without running the full suite. I just have a feeling it has
>>> become more frequent lately.
>>
>> When we are at it... An automated thingy which accepts URL to a Git repo, does
>> all the test magic, and spits out test results without user interaction would
>> be an awesome Christmas present!
>>
>> Bonus points if we can get Github integration so I can just push and have it
>> tested automatically so I cannot forget to do that before sending the patch
>> for review.
> 
> +1. Having basic CI test suite run on top of a Pull Request would be awesome.
> 

I'd be happy with just the ipatests being run manually with each review.

And it's then reviewer that I'm focusing on here. A developer _should_
also run the tests but part of the reviewer's responsibility is to
ensure the patch does what it says it does without breaking things.

rob