[#8622] Clean setup.py code and improve coverage. #467
Conversation
|
This patch will break the twisted-depgraph utility mentioned here: https://twistedmatrix.com/trac/wiki/Plan/Python3 There are some OK cleanups in this patch, but overall this patch seems like a lot |
|
Thanks @rodrigc for the feedback. Much appreciated! I have reverted the dist3 changes. The goal of this branch is to set things right for the code in setup.py and dist.py . A comment is a poor way of warning about a private API... so t.python._setup should follow the Twisted convention for private API dist.py was created to host the code from setup.py so that we can test it, but we ended still having code in setup.py and missing coverage. |
Current coverage is 90.76% (diff: 97.82%)@@ trunk #467 diff @@
==========================================
Files 833 833
Lines 145110 145056 -54
Methods 0 0
Messages 0 0
Branches 13616 13607 -9
==========================================
- Hits 131708 131664 -44
+ Misses 11140 11133 -7
+ Partials 2262 2259 -3
|
|
I see very little value to this patch, and it seems like a lot of unnecessary churn. Better coverage of setup.py can be achieved by running setup.py under the coverage command |
|
|
||
|
|
||
| def getVersion(base): | ||
| def get_setup_args(): |
There was a problem hiding this comment.
I would like this to be def getSetupArgs(extensions=_EXTENSIONS): to fit the coding standard, and mean you can pass _EXTENSIONS in explicitly without monkeypatching.
There was a problem hiding this comment.
True. Thanks! Will update.
| """ | ||
| good_ext = ConditionalExtension("whatever", ["whatever.c"], | ||
| condition=lambda b: True) | ||
| bad_ext = ConditionalExtension("whatever", ["whatever.c"], | ||
| condition=lambda b: False) | ||
| args = get_setup_args(conditionalExtensions=[good_ext, bad_ext]) | ||
| self.patch(_setup, '_EXTENSIONS', [good_ext, bad_ext]) | ||
| args = get_setup_args() |
There was a problem hiding this comment.
Per #467 (comment), this would not have a patch call...
See: https://twistedmatrix.com/trac/ticket/8622