Discussion:
[Twisted-Python] INCOMPATIBLE CHANGE: twisted.python.dist renamed to twisted.python._setup
Adi Roiban
2016-08-07 11:19:43 UTC
Permalink
Hi,

I would like to remove twisted.python.dist as a public API without
following the 1 year deprecation processes.

The module has a comment which informs people not to use it outside of
Twisted, but I think it should be private module instead.

The ticket is here https://twistedmatrix.com/trac/ticket/8622
The PR is here https://github.com/twisted/twisted/pull/467
The branch is here
https://github.com/twisted/twisted/tree/8622-setup-coverage

This is part of the setup.py cleanup effort for having most of the setup.py
code outside of setup.py itself so that we can test it as part of the
automated test suite.

twisted.python.dist3 was kept as a private API as it is already used by
https://github.com/habnabit/twisted-depgraph

Please let me know if you are using twisted.python.dist outside of Twisted
and are against this change or if you approve it.

Thanks!
--
Adi Roiban
Glyph Lefkowitz
2016-08-07 23:15:21 UTC
Permalink
Hi,
I would like to remove twisted.python.dist as a public API without following the 1 year deprecation processes.
The module has a comment which informs people not to use it outside of Twisted, but I think it should be private module instead.
The ticket is here https://twistedmatrix.com/trac/ticket/8622 <https://twistedmatrix.com/trac/ticket/8622>
The PR is here https://github.com/twisted/twisted/pull/467 <https://github.com/twisted/twisted/pull/467>
The branch is here https://github.com/twisted/twisted/tree/8622-setup-coverage <https://github.com/twisted/twisted/tree/8622-setup-coverage>
This is part of the setup.py cleanup effort for having most of the setup.py code outside of setup.py itself so that we can test it as part of the automated test suite.
twisted.python.dist3 was kept as a private API as it is already used by https://github.com/habnabit/twisted-depgraph <https://github.com/habnabit/twisted-depgraph>
Please let me know if you are using twisted.python.dist outside of Twisted and are against this change or if you approve it.
Thanks!
+1 from me!

-g
Craig Rodrigues
2016-08-08 00:15:39 UTC
Permalink
Post by Adi Roiban
Hi,
I would like to remove twisted.python.dist as a public API without
following the 1 year deprecation processes.
The module has a comment which informs people not to use it outside of
Twisted, but I think it should be private module instead.
The ticket is here https://twistedmatrix.com/trac/ticket/8622
The PR is here https://github.com/twisted/twisted/pull/467
The branch is here https://github.com/twisted/twisted/tree/8622-
setup-coverage
This is part of the setup.py cleanup effort for having most of the
setup.py code outside of setup.py itself so that we can test it as part of
the automated test suite.
twisted.python.dist3 was kept as a private API as it is already used by
https://github.com/habnabit/twisted-depgraph
Please let me know if you are using twisted.python.dist outside of Twisted
and are against this change or if you approve it.
Thanks!
+1 from me!
-1 from me. dist.py probably should have been a private API from the
beginning, but it slipped through the cracks.
However, what has been done is done. This change seems gratuitous and
pointless with
no benefits to anybody.

--
Craig
Amber "Hawkie" Brown
2016-08-08 00:25:48 UTC
Permalink
Post by Glyph Lefkowitz
Hi,
I would like to remove twisted.python.dist as a public API without following the 1 year deprecation processes.
The module has a comment which informs people not to use it outside of Twisted, but I think it should be private module instead.
The ticket is here https://twistedmatrix.com/trac/ticket/8622 <https://twistedmatrix.com/trac/ticket/8622>
The PR is here https://github.com/twisted/twisted/pull/467 <https://github.com/twisted/twisted/pull/467>
The branch is here https://github.com/twisted/twisted/tree/8622-setup-coverage <https://github.com/twisted/twisted/tree/8622-setup-coverage>
This is part of the setup.py cleanup effort for having most of the setup.py code outside of setup.py itself so that we can test it as part of the automated test suite.
twisted.python.dist3 was kept as a private API as it is already used by https://github.com/habnabit/twisted-depgraph <https://github.com/habnabit/twisted-depgraph>
Please let me know if you are using twisted.python.dist outside of Twisted and are against this change or if you approve it.
Thanks!
+1 from me!
-1 from me. dist.py probably should have been a private API from the beginning, but it slipped through the cracks.
However, what has been done is done. This change seems gratuitous and pointless with
no benefits to anybody.
--
Craig
I believe there's a point to it -- that is, we make it clear what is and what isn't user API. It's always said it in the header, but we should codify that it's really not user API. Plus it being private means we can make what changes we need for future setup.py changes, without deprecating things that nobody but us should ever use.

+1 from me.

- Amber
Craig Rodrigues
2016-08-08 00:35:24 UTC
Permalink
On Sun, Aug 7, 2016 at 5:25 PM, Amber "Hawkie" Brown <
Post by Craig Rodrigues
Post by Adi Roiban
Hi,
I would like to remove twisted.python.dist as a public API without
following the 1 year deprecation processes.
The module has a comment which informs people not to use it outside of
Twisted, but I think it should be private module instead.
The ticket is here https://twistedmatrix.com/trac/ticket/8622
The PR is here https://github.com/twisted/twisted/pull/467
The branch is here https://github.com/twisted/twisted/tree/8622-setup-
coverage
This is part of the setup.py cleanup effort for having most of the
setup.py code outside of setup.py itself so that we can test it as part of
the automated test suite.
twisted.python.dist3 was kept as a private API as it is already used by
https://github.com/habnabit/twisted-depgraph
Please let me know if you are using twisted.python.dist outside of
Twisted and are against this change or if you approve it.
Thanks!
+1 from me!
-1 from me. dist.py probably should have been a private API from the
beginning, but it slipped through the cracks.
However, what has been done is done. This change seems gratuitous and pointless with
no benefits to anybody.
--
Craig
I believe there's a point to it -- that is, we make it clear what is and
what isn't user API. It's always said it in the header, but we should
codify that it's really not user API. Plus it being private means we can
make what changes we need for future setup.py changes, without deprecating
things that nobody but us should ever use.
+1 from me.
I understand the motivation, but the time to have done this was when dist.py
was originally reviewed.

There are real problems in the Twisted code, and in the Twisted
infrastructure.
Focusing on something like this instead of those problems seems pointless,
gratuitous and of little benefit.

--
Craig
Amber Brown
2016-08-08 00:42:48 UTC
Permalink
t.p.dist dates from 2005, and as far as I can tell, predates Twisted's code
review system. It's an early mistake, but we can fix many mistakes in
parallel.

- Amber
Post by Craig Rodrigues
On Sun, Aug 7, 2016 at 5:25 PM, Amber "Hawkie" Brown <
Post by Craig Rodrigues
Post by Adi Roiban
Hi,
I would like to remove twisted.python.dist as a public API without
following the 1 year deprecation processes.
The module has a comment which informs people not to use it outside of
Twisted, but I think it should be private module instead.
The ticket is here https://twistedmatrix.com/trac/ticket/8622
The PR is here https://github.com/twisted/twisted/pull/467
The branch is here https://github.com/twiste
d/twisted/tree/8622-setup-coverage
This is part of the setup.py cleanup effort for having most of the
setup.py code outside of setup.py itself so that we can test it as part of
the automated test suite.
twisted.python.dist3 was kept as a private API as it is already used by
https://github.com/habnabit/twisted-depgraph
Please let me know if you are using twisted.python.dist outside of
Twisted and are against this change or if you approve it.
Thanks!
+1 from me!
-1 from me. dist.py probably should have been a private API from the
beginning, but it slipped through the cracks.
However, what has been done is done. This change seems gratuitous and pointless with
no benefits to anybody.
--
Craig
I believe there's a point to it -- that is, we make it clear what is and
what isn't user API. It's always said it in the header, but we should
codify that it's really not user API. Plus it being private means we can
make what changes we need for future setup.py changes, without deprecating
things that nobody but us should ever use.
+1 from me.
I understand the motivation, but the time to have done this was when dist.py
was originally reviewed.
There are real problems in the Twisted code, and in the Twisted
infrastructure.
Focusing on something like this instead of those problems seems pointless,
gratuitous and of little benefit.
--
Craig
_______________________________________________
Twisted-Python mailing list
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Tristan Seligmann
2016-08-08 00:58:06 UTC
Permalink
+1 from me
Post by Craig Rodrigues
I understand the motivation, but the time to have done this was when dist.py
was originally reviewed.
There are real problems in the Twisted code, and in the Twisted
infrastructure.
Focusing on something like this instead of those problems seems pointless,
gratuitous and of little benefit.
The problem is that if we defer this change until later, it becomes a lot
more work then. If we do it now, while nobody is depending on the module,
then we save ourselves that work in future by doing a trivial amount of
work now.
Chris Wolfe
2016-08-08 03:00:30 UTC
Permalink
+1 from me as well. Depending directly on how twisted installs itself seems
problematic. In my opinion, the more we discourage this practice the better.
Post by Glyph Lefkowitz
+1 from me
Post by Craig Rodrigues
I understand the motivation, but the time to have done this was when dist.py
was originally reviewed.
There are real problems in the Twisted code, and in the Twisted
infrastructure.
Focusing on something like this instead of those problems seems pointless,
gratuitous and of little benefit.
The problem is that if we defer this change until later, it becomes a lot
more work then. If we do it now, while nobody is depending on the module,
then we save ourselves that work in future by doing a trivial amount of
work now.
_______________________________________________
Twisted-Python mailing list
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
--
- Chris
Glyph Lefkowitz
2016-08-08 06:01:54 UTC
Permalink
Post by Craig Rodrigues
I understand the motivation, but the time to have done this was when dist.py
was originally reviewed.
It certainly would have been better :-).
Post by Craig Rodrigues
There are real problems in the Twisted code, and in the Twisted infrastructure.
This is also addressing a real problem. One of the major issues with Twisted is documentation noise. Developers trying to use Twisted to build things frequently complain about this: they look for an API to do XYZ, and they search the reference documentation, which produces dozens of confusing, irrelevant hits. There are multiple issues that interact here (see, for example, <https://github.com/twisted/pydoctor/issues/49> which is a big part of the problem) but one significant one is that our apparently-public API surface is just too big. Removing things like this, which are really _totally_ useless to anyone, is useful in and of itself and also amplifies the benefit of any other work on docs and tooling to properly segregate public and private API documentation.

For a long time we were extremely conservative about doing any compatibility-breaking thing, even hypothetically breaking, even within the policy we'd set forth, and I think the result of that is a sprawling API that's incredibly hard to navigate, full of old junk that nobody should ever use. The slightly more aggressive attitude that has been exemplified by e.g. Amber has been a breath of fresh air when it comes to unburdening ourselves from literal garbage.

So I think that "overburdened public API surface" is one of the largest problems facing Twisted and every little bit we can chip away at it is a real win. We do have to be super careful of course, we don't want to be chopping off obscure but useful pieces of API that programs were actually using, but that's what the compat policy (and judicious judgement) are for.
Post by Craig Rodrigues
Focusing on something like this instead of those problems seems pointless, gratuitous and of little benefit.
In addition to underplaying the usefulness of maintenance like this, I think this comment belies acceptance of a superficially true falsehood - one that I've often fallen prey to myself - and that is, the idea that volunteer time is fungible. It isn't. If the members of a project have problems A, B, C, X, Y, Z in very clear ranked order of importance, and a volunteer ("V") shows up to work on Z, the impulse of the existing members of the project is "but you should really just work on 'A'!". V doesn't care about A, doesn't want to work on A, and the likely result of telling them they should be working on A is that they will feel bad about not working on A and their fix for Z will go unfinished.

I'm not saying that necessarily applies in its full force here; after all Adi is a member of the project as well. But one of the corollaries of this realization is that if A were really so important someone would already be working on it :).

We should totally have the ABCXYZ list in case anyone shows up just wondering what would be helpful to work on; but if someone shows up with a solution to Z, we should always try to incorporate it and help them feel motivated about continuing, hopefully up the list towards A of course, but maybe they are going to contribute stuff for M, N, and O that we didn't even realize was a problem.

-glyph
Craig Rodrigues
2016-08-08 06:53:50 UTC
Permalink
This is also addressing a real problem. One of the *major* issues with
Twisted is *documentation noise*. Developers trying to use Twisted to
build things frequently complain about this: they look for an API to do
XYZ, and they search the reference documentation, which produces dozens of
confusing, irrelevant hits. There are multiple issues that interact here
(see, for example, <https://github.com/twisted/pydoctor/issues/49> which
is a big part of the problem) but one significant one is that our
apparently-public API surface is just too big. Removing things like this,
which are really _totally_ useless to anyone, is useful in and of itself
and also amplifies the benefit of any other work on docs and tooling to
properly segregate public and private API documentation.
Making dist.py private seems to fall under the category of "code
cleanliness", and doesn't
seem to solve any specific pressing issue that I see. Reducing
documentation noise is a good goal,
but if dist.py was left the way it is, things would probably be OK with
Twisted, and I think people
could figure things out in Twisted with respect to the documentation.

Since you mention pydoctor, I will mention two issues, that I feel are more
important to the project
that moving around dist.py:


- pydoctor produces false errors, specifically in h2 code:
https://buildbot.twistedmatrix.com/builders/documentation/builds/1291/steps/api-documentation/logs/pydoctor%20errors
- pydoctor doesn't work on Python 3:
https://github.com/twisted/pydoctor/issues/96

I understand that Twisted is a volunteer project, and people can work on
whatever they want.
I also see on this list, that a few people support the dist.py change, so
as long as
Adi's change meets the Twisted coding standards, gets code review approval,
and can pass all the buildbots,
it can move forward, despite my objections.

--
Craig
Christopher Armstrong
2016-08-08 07:10:39 UTC
Permalink
The reason this module never had an underscore (and instead has the "don't
use this outside of Twisted" line in the docstring) is because it
originated in a time when Twisted was ostensibly split up into multiple
projects, and I ill-advisedly decided that it technically needed to be
"public" in order for the non-Twisted-core subprojects to use it. That was
a silly decision, and given that the docstring has always told people not
to use it, please feel free to rename it.

-Chris
Post by Craig Rodrigues
This is also addressing a real problem. One of the *major* issues with
Twisted is *documentation noise*. Developers trying to use Twisted to
build things frequently complain about this: they look for an API to do
XYZ, and they search the reference documentation, which produces dozens of
confusing, irrelevant hits. There are multiple issues that interact here
(see, for example, <https://github.com/twisted/pydoctor/issues/49> which
is a big part of the problem) but one significant one is that our
apparently-public API surface is just too big. Removing things like this,
which are really _totally_ useless to anyone, is useful in and of itself
and also amplifies the benefit of any other work on docs and tooling to
properly segregate public and private API documentation.
Making dist.py private seems to fall under the category of "code
cleanliness", and doesn't
seem to solve any specific pressing issue that I see. Reducing
documentation noise is a good goal,
but if dist.py was left the way it is, things would probably be OK with
Twisted, and I think people
could figure things out in Twisted with respect to the documentation.
Since you mention pydoctor, I will mention two issues, that I feel are
more important to the project
https://buildbot.twistedmatrix.com/builders/documentation/builds/1291/steps/api-documentation/logs/pydoctor%20errors
https://github.com/twisted/pydoctor/issues/96
I understand that Twisted is a volunteer project, and people can work on
whatever they want.
I also see on this list, that a few people support the dist.py change, so
as long as
Adi's change meets the Twisted coding standards, gets code review
approval, and can pass all the buildbots,
it can move forward, despite my objections.
--
Craig
_______________________________________________
Twisted-Python mailing list
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
Adi Roiban
2016-08-08 09:54:26 UTC
Permalink
Post by Craig Rodrigues
This is also addressing a real problem. One of the *major* issues with
Twisted is *documentation noise*. Developers trying to use Twisted to
build things frequently complain about this: they look for an API to do
XYZ, and they search the reference documentation, which produces dozens of
confusing, irrelevant hits. There are multiple issues that interact here
(see, for example, <https://github.com/twisted/pydoctor/issues/49> which
is a big part of the problem) but one significant one is that our
apparently-public API surface is just too big. Removing things like this,
which are really _totally_ useless to anyone, is useful in and of itself
and also amplifies the benefit of any other work on docs and tooling to
properly segregate public and private API documentation.
Making dist.py private seems to fall under the category of "code
cleanliness", and doesn't
seem to solve any specific pressing issue that I see. Reducing
documentation noise is a good goal,
but if dist.py was left the way it is, things would probably be OK with
Twisted, and I think people
could figure things out in Twisted with respect to the documentation.
Since you mention pydoctor, I will mention two issues, that I feel are
more important to the project
https://buildbot.twistedmatrix.com/builders/documentation/builds/1291/
steps/api-documentation/logs/pydoctor%20errors
<https://buildbot.twistedmatrix.com/builders/documentation/builds/1291/steps/api-documentation/logs/pydoctor%20errors>
- pydoctor doesn't work on Python 3: https://github.com/twisted/
pydoctor/issues/96
I have already proposed a solution for the pydoctor h2 issue
https://github.com/twisted/pydoctor/pull/126
... waiting for a review :)
--
Adi Roiban
Loading...