INCOMPATIBLE CHANGE: Removal of TestCase.defer{SetUp, TestMethod, TearDown, RunCleanups} methods
Hi, I created this PR to remove TestCase.defer{SetUp, TestMethod, TearDown, RunCleanups} methods: https://github.com/twisted/twisted/pull/12389 The justification is as follows: Currently `TestCase` has the following functions that are presumably public due to their naming: `deferSetUp`, `deferTestMethod`, `deferRunCleanups` and `deferTearDown`. This raises a few problems. I suggest removing the functions without deprecation period due to the following reasons: - these functions expose so much of the internal structure of TestCase that it makes the code hard to change (e.g. https://github.com/twisted/twisted/pull/12375 cannot go in) - there is no case where the functions would be useful as an extension point. The only use of the functions that I found using Github code search is my own code in the Buildbot project which overrides `deferRunCleanups` and hooks even deeper into TestCase internals. - the API is not "public ready" (i.e. the `ignored` parameter) - it seems as if they were some kind of internal thing that simply didn't get the `_` prefix. - deprecation would be harder to implement than usual, due to the functions being supposed to be overridden, not just called. - this is API for tests, so it is not likely to break functionality that is used in production. Only development workflows may be affected where the fix is to update the tests. Given the above - small risk of breaking code for someone and relatively high costs for going through the deprecation dance I would suggest to have an exception for this change and allow to remove the functions without deprecation period. If you have any objections and you think that the functions should stay or at least be removed via deprecation please let me know. You can send your feedback by replying to this email or add your comment to the PR https://github.com/twisted/twisted/pull/12389 Thanks for your time! Regards, Povilas
I'm inclined to agree with this rationale.
On Dec 8, 2024, at 6:32 AM, povilas@radix.lt wrote:
Hi,
I created this PR to remove TestCase.defer{SetUp, TestMethod, TearDown, RunCleanups} methods: https://github.com/twisted/twisted/pull/12389
The justification is as follows:
Currently `TestCase` has the following functions that are presumably public due to their naming: `deferSetUp`, `deferTestMethod`, `deferRunCleanups` and `deferTearDown`. This raises a few problems. I suggest removing the functions without deprecation period due to the following reasons:
- these functions expose so much of the internal structure of TestCase that it makes the code hard to change (e.g. https://github.com/twisted/twisted/pull/12375 cannot go in)
- there is no case where the functions would be useful as an extension point. The only use of the functions that I found using Github code search is my own code in the Buildbot project which overrides `deferRunCleanups` and hooks even deeper into TestCase internals.
"it appears that nobody is using it at all" is the most convincing exception argument - eventually the policy is meant to support real users, and although not *all* of our users are public, if their code is not public _and_ they're not on this mailing list, we can't really expect to serve them. Thanks for checking through the github corpus!
- the API is not "public ready" (i.e. the `ignored` parameter) - it seems as if they were some kind of internal thing that simply didn't get the `_` prefix.
- deprecation would be harder to implement than usual, due to the functions being supposed to be overridden, not just called.
I wonder if we could do anything with our deprecation helpers to make this easier to manage? Inheritance does make this all unnecessarily more difficult.
- this is API for tests, so it is not likely to break functionality that is used in production. Only development workflows may be affected where the fix is to update the tests.
This point isn't particularly convincing since the idea with the deprecation policy is that you can run your tests against the new version and get legible warning messages that you can correct; if we broke test functionality then you end up stuck.
Given the above - small risk of breaking code for someone and relatively high costs for going through the deprecation dance I would suggest to have an exception for this change and allow to remove the functions without deprecation period.
If you have any objections and you think that the functions should stay or at least be removed via deprecation please let me know.
You can send your feedback by replying to this email or add your comment to the PR
https://github.com/twisted/twisted/pull/12389
Thanks for your time!
Regards, Povilas
-g
Hi Povilas, Many thanks for working on this and following up with the deprecation exception process. +1 for approving the exception. On Sun, 8 Dec 2024 at 21:37, <povilas@radix.lt> wrote:
Hi,
I created this PR to remove TestCase.defer{SetUp, TestMethod, TearDown, RunCleanups} methods: https://github.com/twisted/twisted/pull/12389
The justification is as follows:
Currently `TestCase` has the following functions that are presumably public due to their naming: `deferSetUp`, `deferTestMethod`, `deferRunCleanups` and `deferTearDown`. This raises a few problems. I suggest removing the functions without deprecation period due to the following reasons:
- these functions expose so much of the internal structure of TestCase that it makes the code hard to change (e.g. https://github.com/twisted/twisted/pull/12375 cannot go in)
- there is no case where the functions would be useful as an extension point. The only use of the functions that I found using Github code search is my own code in the Buildbot project which overrides `deferRunCleanups` and hooks even deeper into TestCase internals.
- the API is not "public ready" (i.e. the `ignored` parameter) - it seems as if they were some kind of internal thing that simply didn't get the `_` prefix.
- deprecation would be harder to implement than usual, due to the functions being supposed to be overridden, not just called.
- this is API for tests, so it is not likely to break functionality that is used in production. Only development workflows may be affected where the fix is to update the tests.
Given the above - small risk of breaking code for someone and relatively high costs for going through the deprecation dance I would suggest to have an exception for this change and allow to remove the functions without deprecation period.
If you have any objections and you think that the functions should stay or at least be removed via deprecation please let me know.
You can send your feedback by replying to this email or add your comment to the PR
https://github.com/twisted/twisted/pull/12389
Thanks for your time!
Regards, Povilas _______________________________________________ Twisted mailing list -- twisted@python.org To unsubscribe send an email to twisted-leave@python.org https://mail.python.org/mailman3/lists/twisted.python.org/ Message archived at https://mail.python.org/archives/list/twisted@python.org/message/QOHSOCN45LI... Code of Conduct: https://twisted.org/conduct
-- Adi Roiban
Why not add a breaking changes locked (to contributors with write access) and pinned issue that people can subscribe to like what trio does? Eg, https://github.com/python-trio/trio/issues/1 On Mon, Dec 9, 2024, 6:32 AM Adi Roiban via Twisted <twisted@python.org> wrote:
Hi Povilas,
Many thanks for working on this and following up with the deprecation exception process.
+1 for approving the exception.
On Sun, 8 Dec 2024 at 21:37, <povilas@radix.lt> wrote:
Hi,
I created this PR to remove TestCase.defer{SetUp, TestMethod, TearDown, RunCleanups} methods: https://github.com/twisted/twisted/pull/12389
The justification is as follows:
Currently `TestCase` has the following functions that are presumably public due to their naming: `deferSetUp`, `deferTestMethod`, `deferRunCleanups` and `deferTearDown`. This raises a few problems. I suggest removing the functions without deprecation period due to the following reasons:
- these functions expose so much of the internal structure of TestCase that it makes the code hard to change (e.g. https://github.com/twisted/twisted/pull/12375 cannot go in)
- there is no case where the functions would be useful as an extension point. The only use of the functions that I found using Github code search is my own code in the Buildbot project which overrides `deferRunCleanups` and hooks even deeper into TestCase internals.
- the API is not "public ready" (i.e. the `ignored` parameter) - it seems as if they were some kind of internal thing that simply didn't get the `_` prefix.
- deprecation would be harder to implement than usual, due to the functions being supposed to be overridden, not just called.
- this is API for tests, so it is not likely to break functionality that is used in production. Only development workflows may be affected where the fix is to update the tests.
Given the above - small risk of breaking code for someone and relatively high costs for going through the deprecation dance I would suggest to have an exception for this change and allow to remove the functions without deprecation period.
If you have any objections and you think that the functions should stay or at least be removed via deprecation please let me know.
You can send your feedback by replying to this email or add your comment to the PR
https://github.com/twisted/twisted/pull/12389
Thanks for your time!
Regards, Povilas _______________________________________________ Twisted mailing list -- twisted@python.org To unsubscribe send an email to twisted-leave@python.org https://mail.python.org/mailman3/lists/twisted.python.org/ Message archived at https://mail.python.org/archives/list/twisted@python.org/message/QOHSOCN45LI... Code of Conduct: https://twisted.org/conduct
-- Adi Roiban _______________________________________________ Twisted mailing list -- twisted@python.org To unsubscribe send an email to twisted-leave@python.org https://mail.python.org/mailman3/lists/twisted.python.org/ Message archived at https://mail.python.org/archives/list/twisted@python.org/message/HSH2FK4Y4EW... Code of Conduct: https://twisted.org/conduct
Because it's a bunch of work and process change that wouldn't solve any particular problem? If you could restate this as a problem you think we have with this process, maybe there would be some cost-benefit to discuss :) -g
On Dec 8, 2024, at 11:17 PM, Thomas Grainger via Twisted <twisted@python.org> wrote:
Why not add a breaking changes locked (to contributors with write access) and pinned issue that people can subscribe to like what trio does? Eg, https://github.com/python-trio/trio/issues/1
On Mon, Dec 9, 2024, 6:32 AM Adi Roiban via Twisted <twisted@python.org <mailto:twisted@python.org>> wrote:
Hi Povilas,
Many thanks for working on this and following up with the deprecation exception process.
+1 for approving the exception.
On Sun, 8 Dec 2024 at 21:37, <povilas@radix.lt <mailto:povilas@radix.lt>> wrote:
Hi,
I created this PR to remove TestCase.defer{SetUp, TestMethod, TearDown, RunCleanups} methods: https://github.com/twisted/twisted/pull/12389
The justification is as follows:
Currently `TestCase` has the following functions that are presumably public due to their naming: `deferSetUp`, `deferTestMethod`, `deferRunCleanups` and `deferTearDown`. This raises a few problems. I suggest removing the functions without deprecation period due to the following reasons:
- these functions expose so much of the internal structure of TestCase that it makes the code hard to change (e.g. https://github.com/twisted/twisted/pull/12375 cannot go in)
- there is no case where the functions would be useful as an extension point. The only use of the functions that I found using Github code search is my own code in the Buildbot project which overrides `deferRunCleanups` and hooks even deeper into TestCase internals.
- the API is not "public ready" (i.e. the `ignored` parameter) - it seems as if they were some kind of internal thing that simply didn't get the `_` prefix.
- deprecation would be harder to implement than usual, due to the functions being supposed to be overridden, not just called.
- this is API for tests, so it is not likely to break functionality that is used in production. Only development workflows may be affected where the fix is to update the tests.
Given the above - small risk of breaking code for someone and relatively high costs for going through the deprecation dance I would suggest to have an exception for this change and allow to remove the functions without deprecation period.
If you have any objections and you think that the functions should stay or at least be removed via deprecation please let me know.
You can send your feedback by replying to this email or add your comment to the PR
https://github.com/twisted/twisted/pull/12389
Thanks for your time!
Regards, Povilas _______________________________________________ Twisted mailing list -- twisted@python.org <mailto:twisted@python.org> To unsubscribe send an email to twisted-leave@python.org <mailto:twisted-leave@python.org> https://mail.python.org/mailman3/lists/twisted.python.org/ Message archived at https://mail.python.org/archives/list/twisted@python.org/message/QOHSOCN45LI... Code of Conduct: https://twisted.org/conduct
-- Adi Roiban _______________________________________________ Twisted mailing list -- twisted@python.org <mailto:twisted@python.org> To unsubscribe send an email to twisted-leave@python.org <mailto:twisted-leave@python.org> https://mail.python.org/mailman3/lists/twisted.python.org/ Message archived at https://mail.python.org/archives/list/twisted@python.org/message/HSH2FK4Y4EW... Code of Conduct: https://twisted.org/conduct
Twisted mailing list -- twisted@python.org To unsubscribe send an email to twisted-leave@python.org https://mail.python.org/mailman3/lists/twisted.python.org/ Message archived at https://mail.python.org/archives/list/twisted@python.org/message/WFQV3L5YRJ7... Code of Conduct: https://twisted.org/conduct
The problem is some people don't want to subscribe to mailing lists, or send a response if something is breaking, but are happy to subscribe to a GitHub issue and leave an emoji response. The issue should only be for merged breaking changes, not discussion of the possibility of breaking changes like the mailing list On Mon, Dec 9, 2024, 7:23 AM Glyph <glyph@twistedmatrix.com> wrote:
Because it's a bunch of work and process change that wouldn't solve any particular problem? If you could restate this as a problem you think we have with this process, maybe there would be some cost-benefit to discuss :)
-g
On Dec 8, 2024, at 11:17 PM, Thomas Grainger via Twisted < twisted@python.org> wrote:
Why not add a breaking changes locked (to contributors with write access) and pinned issue that people can subscribe to like what trio does? Eg, https://github.com/python-trio/trio/issues/1
On Mon, Dec 9, 2024, 6:32 AM Adi Roiban via Twisted <twisted@python.org> wrote:
Hi Povilas,
Many thanks for working on this and following up with the deprecation exception process.
+1 for approving the exception.
On Sun, 8 Dec 2024 at 21:37, <povilas@radix.lt> wrote:
Hi,
I created this PR to remove TestCase.defer{SetUp, TestMethod, TearDown, RunCleanups} methods: https://github.com/twisted/twisted/pull/12389
The justification is as follows:
Currently `TestCase` has the following functions that are presumably public due to their naming: `deferSetUp`, `deferTestMethod`, `deferRunCleanups` and `deferTearDown`. This raises a few problems. I suggest removing the functions without deprecation period due to the following reasons:
- these functions expose so much of the internal structure of TestCase that it makes the code hard to change (e.g. https://github.com/twisted/twisted/pull/12375 cannot go in)
- there is no case where the functions would be useful as an extension point. The only use of the functions that I found using Github code search is my own code in the Buildbot project which overrides `deferRunCleanups` and hooks even deeper into TestCase internals.
- the API is not "public ready" (i.e. the `ignored` parameter) - it seems as if they were some kind of internal thing that simply didn't get the `_` prefix.
- deprecation would be harder to implement than usual, due to the functions being supposed to be overridden, not just called.
- this is API for tests, so it is not likely to break functionality that is used in production. Only development workflows may be affected where the fix is to update the tests.
Given the above - small risk of breaking code for someone and relatively high costs for going through the deprecation dance I would suggest to have an exception for this change and allow to remove the functions without deprecation period.
If you have any objections and you think that the functions should stay or at least be removed via deprecation please let me know.
You can send your feedback by replying to this email or add your comment to the PR
https://github.com/twisted/twisted/pull/12389
Thanks for your time!
Regards, Povilas _______________________________________________ Twisted mailing list -- twisted@python.org To unsubscribe send an email to twisted-leave@python.org https://mail.python.org/mailman3/lists/twisted.python.org/ Message archived at https://mail.python.org/archives/list/twisted@python.org/message/QOHSOCN45LI... Code of Conduct: https://twisted.org/conduct
-- Adi Roiban _______________________________________________ Twisted mailing list -- twisted@python.org To unsubscribe send an email to twisted-leave@python.org https://mail.python.org/mailman3/lists/twisted.python.org/ Message archived at https://mail.python.org/archives/list/twisted@python.org/message/HSH2FK4Y4EW... Code of Conduct: https://twisted.org/conduct
_______________________________________________ Twisted mailing list -- twisted@python.org To unsubscribe send an email to twisted-leave@python.org https://mail.python.org/mailman3/lists/twisted.python.org/ Message archived at https://mail.python.org/archives/list/twisted@python.org/message/WFQV3L5YRJ7... Code of Conduct: https://twisted.org/conduct
_______________________________________________ Twisted mailing list -- twisted@python.org To unsubscribe send an email to twisted-leave@python.org https://mail.python.org/mailman3/lists/twisted.python.org/ Message archived at https://mail.python.org/archives/list/twisted@python.org/message/6N4IIVZSIGH... Code of Conduct: https://twisted.org/conduct
On Dec 8, 2024, at 11:26 PM, Thomas Grainger via Twisted <twisted@python.org> wrote: The problem is some people don't want to subscribe to mailing lists, or send a response if something is breaking, but are happy to subscribe to a GitHub issue and leave an emoji response.
The issue should only be for merged breaking changes, not discussion of the possibility of breaking changes like the mailing list
I guess I'm open to this idea if it's something folks actively want, but as it stands, I would prefer not to do this. The main thing that it lets users do is engage with the Twisted community even less than they are right now. If we had a big problem with traffic on this mailing list getting to the point of it being spammy, I'd be keen to address that, but if anything we have the opposite problem — very little feedback from users, very little conversation, most of which has been shunted off to specific issues, so it's extremely difficult to get a sense of what's going on. There are really only two cases for notifications of incompatible changes: either you want to be involved, or you don't. If you want to be involved, then you need to be willing to type out a sentence or two when you want to avoid an incompatible change landing. I don't think that emoji reactions are usually sufficient for that level of participation. If you don't want to be involved, i.e. you just want to find out when Twisted has broken an API, at a point past the end of the conversation where it's already been made, then run your tests and see the errors. Simple enough; if you pin your dependencies and upgrade via dependabot or something like it, fix the tests whenever you see a failure and you should be fine. The third case, the one where someone want to be just involved enough to get a pre-notification that they might have to do some work when the next release hits, and then have no recourse but to find some other channel to yell at maintainers to revert already-decided-upon work because that might be easier for them than actually committing to the upgrade, is a kind of user I think we don't want. This is also not really a character flaw from users, it is setting them up for failure, a mode of engagement where the only responses are "nothing" and "toxicity". Anyway, this process is so rarely used that I don't think it bears this many words, but it's worth thinking about user engagement more broadly, and if a change like this were to fit into a more general strategy I'd definitely be interested to hear about it. Much as I wish we had more general conversations about Twisted's development and where it is going, I also have no illusions that mailing lists are the state of the art as far as such things go :). -g
participants (4)
-
Adi Roiban -
Glyph -
povilas＠radix.lt -
Thomas Grainger