Ensured all methods in setuptools.modified raise a consistant distutils.error.DistutilsError type#4567
Conversation
2001168 to
322e6bb
Compare
a7a791d to
d6db114
Compare
d6db114 to
07b26e7
Compare
a08af03 to
439ec5a
Compare
ece7519 to
5759d67
Compare
| @@ -0,0 +1 @@ | |||
| Ensured all methods in ``setuptools.modified`` raise a consistant ``distutils.errors.DistutilsError`` type -- by :user:`Avasam` | |||
There was a problem hiding this comment.
| Ensured all methods in ``setuptools.modified`` raise a consistant ``distutils.errors.DistutilsError`` type -- by :user:`Avasam` | |
| Ensured methods in ``setuptools.modified`` preferably raise a consistent | |
| ``distutils.errors.DistutilsError`` type | |
| (except in the deprecated use case of ``SETUPTOOLS_USE_DISTUTILS=stdlib``) | |
| -- by :user:`Avasam` |
There was a problem hiding this comment.
In the case of the exception (e.g.
SETUPTOOLS_USE_DISTUTILS=stdlib) these methods would raisesetuptools._distutils.error.DistutilsErrorerror, right?So maybe we have to change the news fragment a bit...
hmm, looking at my tests I think the intention was to be truly the same as distutils.errors.DistutilsError at all time, but I see how that won't be the case, unless we start wrapping _distutils methods here to catch & rethrow (let's not do that).
I think that's still an improvement for the "intended usage" (ie: not using deprecated SETUPTOOLS_USE_DISTUTILS=stdlib). But I'll need to update my tests to reflect that.
There was a problem hiding this comment.
I think it is OK to say: "If you need to catch that error, please don't use the deprecated SETUPTOOLS_USE_DISTUTILS=stdlib settings", so I am good with this.
56a626c to
f1d59cc
Compare
…utils.error.DistutilsError` type
Co-authored-by: Anderson Bravalheri <andersonbravalheri+github@gmail.com>
f1d59cc to
82aef16
Compare
eb3a95d to
250b622
Compare
|
@abravalheri btw this can be reduced to a single commit. No need to keep commit history here. |
|
Thank you! |
Summary of changes
The following code should now work as expected in all cases:
I'm not entirely certain my test is correct. Hopefully it should be.
Pull Request Checklist
newsfragments/.(See documentation for details)