Open Exirel opened 1 year ago
Summarizing short discussion from IRC: 8.0 is very long in the tooth now, it seems like we should at-most include a note in the docs about our intent to get rid of SopelWrapper
Eventually™ for 8.0 with no code changes, and we can figure out the rest of the details when 8.0 is out the door
@dgw suggested 9.0 for release of the initial implementation of a contextvars replacement, which I think would push the removal of SopelWrapper
to 10.0, but we can discuss more as we go.
@dgw suggested 9.0 for release of the initial implementation of a contextvars replacement, which I think would push the removal of
SopelWrapper
to 10.0, but we can discuss more as we go.
Thought I clarified in a follow-up line that I meant the switchover would happen in 9.0, with the implementation first shipping in 8.1, but maybe that got lost. x)
With the Issues preview, newly added sub-issues of this one can live in the intermediate milestone(s) like 8.1.0. This one can move to 9.0.0 so it doesn't clutter the 8.1 list.
Requested Feature
This is a long term plan: the end goal is to remove
SopelWrapper
by usingcontextvars
in theSopel
class itself. The main issue is that removing the SopelWrapper may break the following cases:isinstance
will breakSopelWrapper
will obviously fail, andmypy
will complainAll these are perfectly acceptable for a major release, but shouldn't be introduced in a minor release. We should also ensure a period of deprecation, with warning in the documentation, and if possible in the code itself.
After a conversation on IRC, a rough plan is:
SopelWrapper
2521 (target version for removal unspecified for now, but probably 9.0)
Problems Solved
Today, we use
SopelWrapper
to wrap an instance ofSopel
and use it in plugin callable (thebot
argument), and as a result:Sopel
, andSopelWrapper
)Sopel
we have to consider how it will or can be used withSopelWrapper
, requiring more code, more tests, maybe more documentation, etc.SopelWrapper
object, aSopel
object, or both, which can be confusingNotes
PR #2443 started the conversation around
contextvars
and removingSopelWrapper
, but making such a drastic change in Sopel 8 is too much, hence this issue with a proper plan with a target version as Sopel 9.0.