[Twisted-Python] my branches merged by you (exarkun)

[exarkun at twistedmatrix.com]

On 04:26 pm, wolfgang.kde at rohdewald.de wrote:
>Am Sonntag, 5. Oktober 2014, 14:20:35 schrieb 
>exarkun at twistedmatrix.com:
>> >>I noticed that you also changed 'x' to b'x' in some places. I did
>> >>not do that yet because it is not needed for those tickets, I wanted
>> >>to keep them small and focussed.
>>
>>I think I only did this where new uses of 'x' was being introduced to
>>represent a bytes object.
>
>Then I would not have mentioned this. Three such changes in svn r43220.
>b'remote' and two banana encoded strings. I knowingly left it at 
>'remote'
>because I wanted to change that everywhere together.

Hm.  You're right on two counts.  The lines:

    undecodable = b'\x7f' * 65 + b'\x85'
and

    decodable = b'\x01\x81'

were not related to the change.  I'm not sure why I decided to make 
those changes.  If I were doing it again now, I wouldn't.

But I'll stick by the decision to write `b'remote'` instead of 
`'remote'` on the line:

    vocab = b'remote'
>>My goal is for new code being added to be as
>>correct as possible so that it doesn't increase the burden of making
>>these fixes later.  The time when the code is being first introduced 
>>is
>>the time when it's easiest to make sure these things are correct.
>
>I fully agree. Excluding cases where you just re-use an existing string
>in one more place because then you have both 'remote' and b'remote'
>in the source until you change them all (for that you have to grep
>everything anyway). On the other hand, not adapting existing code
>to changed guidelines right when they are changed always generates
>a lot of such inconsistencies, so this one is really peanuts.
>(I know - more manpower is always needed). This is actually a small
>problem for me because I rather copy the style of existing code
>instead of reading boring guidelines.

I'm glad we're mostly in agreement and I'll take that as a win. :)
>
>> >>if possible __execute__ (not __define__) only one assert per test
>>
>>Hmm.  Yes?  I'm not sure.  What do you have in mind here?
>
>svn r43207 Split up the test with multiple assertions into several 
>tests
>
>There you unfolded the loop. If the real reason was the different
>constructions of the type names (like "__builtin__.type"), I would
>have expected a different commit text, and of course you could have
>left the loop, like
>
>for obj, name in ((type, '__builtin__.typ'), ...)
>
>So I assumed that you really want as few assertions executed in one
>test as possible. I cannot think of any other reason than those two.

Ah.  I understand now, and you divined my intent correctly.
>BTW your commit "epytext" fixes most but not all. There are still a
>few ``xxx`` left. What markup language did you have in mind there?

sigh... reStructuredText... which Twisted uses for the other half of its 
documentation.
>> >>Document and isolate test code using the private API of the module 
>>to
>> >>be tested.
>>
>>I'm also unsure about this one.
>
>Cases like svn r43221
>
>Hoist the use of this private API to a single location in a helper 
>function.
>
>which explains why we need to use the private API.

Ah.  Great.

Jean-Paul