Re: [Twisted-Python] my branches merged by you (exarkun)
That should have been exarkun, not itamar. Sorry for that. Am Sonntag, 5. Oktober 2014, 11:59:40 schrieb Wolfgang Rohdewald: Thanks for fixing my mistakes! Based on your fixes I started a personal checklist, see below. Is that list correct so far? (it will not include things I already did right) 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. Instead I am planning tickets for just that: (after minimizing the number of places to be fixed) twisted.spread.banana and tests: literal strings become bytes where appropriate twisted.spread.jelly and tests: define *_atom consistently for all atoms, change them to bytes twisted.spread.pb and tests: literal strings become bytes where appropriate The current checklist: Line length max. 79 *.rst doc: One line per sentence, no length limit fully qualified names in doc and news, twisted.* no __ in Twisted. Everything in a test case is private anyway. if possible __execute__ (not __define__) only one assert per test document changed API like raising more or different exceptions use new print() formatting syntax Document and isolate test code using the private API of the module to be tested. test everything with PY2.6 too did twistedchecker add new warnings? news: most frequent form: Qualified now does ... news about spread belong into twisted/topfiles doc: not pb but PB Epydoc docstring ends with point break docstring lines not sooner than needed avoid the word “test” in test docstrings do not use the “I am” form in docstrings test docstring: describe what happens, including exceptions doc: avoid mentioning the private API -- Wolfgang ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] my branches merged by you (exarkun)
On 12:00 pm, wolfgang@rohdewald.de wrote: That should have been exarkun, not itamar. Sorry for that. Am Sonntag, 5. Oktober 2014, 11:59:40 schrieb Wolfgang Rohdewald: Thanks for fixing my mistakes! Based on your fixes I started a personal checklist, see below. Is that list correct so far? (it will not include things I already did right) 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. 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. Instead I am planning tickets for just that: (after minimizing the number of places to be fixed) twisted.spread.banana and tests: literal strings become bytes where appropriate twisted.spread.jelly and tests: define *_atom consistently for all atoms, change them to bytes twisted.spread.pb and tests: literal strings become bytes where appropriate The current checklist: Line length max. 79 *.rst doc: One line per sentence, no length limit fully qualified names in doc and news, twisted.* no __ in Twisted. Everything in a test case is private anyway. if possible __execute__ (not __define__) only one assert per test Hmm. Yes? I'm not sure. What do you have in mind here? document changed API like raising more or different exceptions use new print() formatting syntax Document and isolate test code using the private API of the module to be tested. I'm also unsure about this one. test everything with PY2.6 too did twistedchecker add new warnings? news: most frequent form: Qualified now does ... news about spread belong into twisted/topfiles doc: not pb but PB Epydoc docstring ends with point break docstring lines not sooner than needed avoid the word “test” in test docstrings do not use the “I am” form in docstrings test docstring: describe what happens, including exceptions doc: avoid mentioning the private API Apart from those two items, the rest of these look great. Jean-Paul ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] my branches merged by you (exarkun)
Am Sonntag, 5. Oktober 2014, 14:20:35 schrieb exar...@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. 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. 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. 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? 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. -- Wolfgang ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Re: [Twisted-Python] my branches merged by you (exarkun)
On 04:26 pm, wolfgang@rohdewald.de wrote: Am Sonntag, 5. Oktober 2014, 14:20:35 schrieb exar...@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 ___ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
