Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sopel: fix TODOs related to Python 2 #2326

Merged
merged 3 commits into from
Aug 3, 2022
Merged

sopel: fix TODOs related to Python 2 #2326

merged 3 commits into from
Aug 3, 2022

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Jul 17, 2022

Description

Tin. Fix parts of #2120 related to Python 2.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

@Exirel Exirel added Breaking Change Stuff that probably should be mentioned in a migration guide Housekeeping Code cleanup, removal of deprecated stuff, etc. labels Jul 17, 2022
@Exirel Exirel added this to the 8.0.0 milestone Jul 17, 2022
@Exirel Exirel mentioned this pull request Jul 17, 2022
Closed
3 tasks
dgw
dgw reviewed Jul 24, 2022
View reviewed changes
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a very big diff considering that scary comment about moving plugin loading to importlib being non-trivial. How well-tested is this?

@Exirel
Copy link
Contributor Author

Exirel commented Jul 24, 2022

How well-tested is this?

Pretty well, if you look at coverage. https://coveralls.io/builds/50929691/source?filename=sopel%2Fplugins%2Fhandlers.py#L417

I spent some time making sure it worked and that there was tests.

@dgw
Copy link
Member

dgw commented Jul 24, 2022

You've tried it "live" in addition to running the test suite? (Just being specific, for documentation reasons.)

dgw
dgw approved these changes Aug 3, 2022
View reviewed changes
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Barring Ex stating explicitly that he's tried this patch live, I'll just merge it into a test branch on our Official™ instance at Libera and see if any of the external plugins there explode.

@dgw dgw merged commit 0479737 into sopel-irc:master Aug 3, 2022
@Exirel Exirel deleted the remove-python2-todos branch April 8, 2023 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgw dgw dgw approved these changes

Assignees
No one assigned
Labels
Breaking Change Stuff that probably should be mentioned in a migration guide Housekeeping Code cleanup, removal of deprecated stuff, etc.
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@Exirel @dgw