[Twisted-Python] my branches merged by you (exarkun)
exarkun at twistedmatrix.com
exarkun at twistedmatrix.com
Sun Oct 5 14:17:49 MDT 2014
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
More information about the Twisted-Python
mailing list