[Twisted-Python] Need clarification on reviews for Python 3 fixes for Twisted

Glyph glyph at twistedmatrix.com
Sat May 28 01:48:31 MDT 2016


> On May 27, 2016, at 13:46, Glyph <glyph at twistedmatrix.com> wrote:
> 
>> On May 27, 2016, at 06:31, Adi Roiban <adi at roiban.ro <mailto:adi at roiban.ro>> wrote:
>> 
>> On 27 May 2016 at 13:13, Itamar Turner-Trauring <itamar at itamarst.org <mailto:itamar at itamarst.org>> wrote:
>> 2. They're doing one particular incompatibility at a time, rather than "here's an assortment of random changes to a module that may or may not port that module fully, who knows."
>> 
>> Some code parts don't have python 2.7 coverage . 
>> Is is still acceptable to touch that code ? :)
> 
> No.  Test coverage is how we know that the behavior is the same on both versions of Python and we're not just hoping that it is.

I was on a plane when I wrote this, and it may have been a bit overly terse.  Let me expand a little bit.

The lines of code in a diff must be covered.  However, if one is porting, let's say, a 2000-line module with no test coverage to be importable under python 3, and 8 lines of code in that module need to change, *only those 8 lines must be covered*.  Obviously, the tests should be as reasonable as possible and should not do anything too evil and gross to get those lines covered, but the author of the change hitting those lines is not responsible for a ground-up comprehensive test suite design for the entire module.  They just need to write the bare minimum test coverage to ensure their changes are covered at all.  If you're making syntax changes, you also don't need to worry about getting those tests to actually pass on python3; it would be great to make smaller, individual changes which do the syntax first, then later actually start adding tests to the py3 suite.

So please don't take this requirement for test coverage to mean that you have to embark on a major project to land these syntax fixes.  It would be great to have syntax fixes like this, and a bare minimum of very simple test coverage, just enough to exercise the lines in question, is perfectly adequate for such a small mechanical change.

-glyph

-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20160528/fab68390/attachment-0002.html>


More information about the Twisted-Python mailing list