On Wed, Mar 3, 2021 at 3:08 AM Glyph <gl...@twistedmatrix.com> wrote:
> > On Mar 2, 2021, at 6:10 PM, Jean-Paul Calderone <exar...@twistedmatrix.com> > wrote: > > Policy aside, this change doesn't seem like much of an improvement to me. > If I were to guess, I would guess the change was made to satisfy some check > Mypy is now being asked to make about Twisted. If that's the case, it > seems unfortunate that real-world software is suffering so that a synthetic > goal can be achieved. I do recognize there is a perception that practical > value can come from attending to the errors Mypy reports. It would > probably benefit everyone if more care were taken to consider the > real-world consequences of changes that are made to satisfy the > non-real-world goalposts set by tools like Mypy. > > > I do think that Mypy was likely highlighting a real issue here, and it > should have been fixed. We had documented interfaces already, and we were > failing to adhere to them properly, and Mypy was complaining about that. > > For easy reference, the change that made these fixes is here > https://github.com/twisted/twisted/pull/1298 > > This genre of fix is definitely something we should unwind over time, > although it does make it easier to start mypy-clean rather than have > hundreds of exclusion lines that need to be manually adjusted, so on > balance I agree with this tradeoff at the point it was made. > Broadly, I agree. But not with this part. It seems like there is clearly a trade-off that is better for everyone. The trade-off represented by #1298: - Breaks application code without providing any new functionality or fixing any bugs - Captures a long list of TODOs as an arbitrarily complex collection of sources spread across the entire codebase - All the work of addressing the problems still remains to be done Contrast this with any basic ratchet-style approach (for example, capture the list of mypy errors, then allow any PR that either removes some or doesn't add any new ones): - At the outset, no application code breaks because nothing has actually changed - As work towards *fixing* the TODOs progresses, if it *does* break application code then at least applications have a chance at some benefit - As the work towards fixing the TODOs progresses, maintainers can easily look up what issues remain by consulting the list of errors that feeds the ratchet-style job. The exact mechanism for the ratchet isn't the point here. Maybe Mypy plays more nicely with inline comments telling it to ignore something, I don't know. The point is: - A centralized list of stuff left to do is better than a mess smeared across the whole codebase - If we're going to risk breaking applications we should at least try to offer some value to them - We shouldn't make applications deal with every change twice when we could just disturb them once Jean-Paul > Specifically what I mean by "this genre of fix" is that you can always > make mypy happy by transforming code like this: > > class Thing: > pass > > Thing().stuff() > > > into code like this: > > class Thing: > @property > def stuff(self): > raise AttributeError("no such thing as `stuff`") > > Thing().stuff() > > > The runtime behavior is the same, but now Mypy can't tell you about this > class of bug any more. > > So, at some point, we should slowly unwind all `NotImplementedError` > methods and find ways to either implement them for real or make their > required use raise at import time. > > Finally: let's not develop an aversion to new tooling and change because > it might create problems; experience over the last few years has shown me > that Mypy can catch *tons* of real bugs and it's well worth getting the > codebase to type check. If we want to prevent breakages like this in the > future, the answer is not to stop trying to get linters and typecheckers to > run cleanly with arbitrary changes, but to figure out some kind of *continuous > integration *solution that's actually continuous with our downstream > dependencies > > If dependencies could start testing against Twisted trunk in some > capacity, we could get notified close to when unintentionally breaking > changes occur, and dependencies can let us know well before the release > happens, and we can either revert or they can fix things if the error is on > their end. If initially, say, crossbar and matrix would like to work with > us to set up some kind of repeatable pattern we can suggest to others, that > would be great. > > -g > _______________________________________________ > Twisted-Python mailing list > Twisted-Python@twistedmatrix.com > https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python >
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com https://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
