Skip to content

plugins: add more type annotations#2634

Draft
dgw wants to merge 2 commits intomasterfrom
typecheck-plugins-submodule
Draft

plugins: add more type annotations#2634
dgw wants to merge 2 commits intomasterfrom
typecheck-plugins-submodule

Conversation

@dgw
Copy link
Copy Markdown
Member

@dgw dgw commented Oct 18, 2024

Description

This does what it says on the tin, and is intended to land in 8.1.0.

Starting as a draft, because it has existing merge conflicts already (I think they hail from #2616, but haven't checked git blame for all of them) and also includes part of #2633 (which needs to land in 8.0.1 instead).

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 lint and make test)
  • I have tested the functionality of the things this change touches

Notes

I probably didn't open a PR from this branch last month because I wasn't sure it was done, but now that a good 4 weeks went by and seeing this in the list of branches made me think, "Wait, what's that for?" I think I'd better get in the habit of opening draft PRs sooner rather than later. #2633 was another forgotten branch that I thought had been turned into a PR but hadn't yet.

dgw added 2 commits September 17, 2024 17:55
In `PyFilePlugin.__init__()`, the `filename` must be wrapped in a list
before passing it to `importlib.util.spec_from_file_location()`, whose
`submodule_search_locations` argument is expected to be a list of `str`.

That +1/-1 patch should be released as a hotfix for 8.0.x, but the rest
of the type changes and doc tweaks can wait for 8.1.
@dgw dgw added the Housekeeping Code cleanup, removal of deprecated stuff, etc. label Oct 18, 2024
@dgw dgw added this to the 8.1.0 milestone Oct 18, 2024
Copy link
Copy Markdown
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

I really really like that. Only a nitpick.

Comment thread sopel/plugins/rules.py
# core
self._regexes = regexes
self._plugin_name = plugin
self._plugin_name = plugin or ''
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Given plugin is always a str now, there is no need for that (unless mypy complain? but that sounds weird):

Suggested change
self._plugin_name = plugin or ''
self._plugin_name = plugin

Note: I'm not even sure why I made plugin optional in the first place.

@dgw
Copy link
Copy Markdown
Member Author

dgw commented Sep 8, 2025

Hmm, I'm torn whether to resolve the conflicts on this. It's doing some unique stuff, but perhaps it's better to wait until after #2669 is done mucking about in plugins/ and then go through .handlers & .rules for typing stuff again with fresh eyes.

@Exirel
Copy link
Copy Markdown
Contributor

Exirel commented Sep 8, 2025

perhaps it's better to wait until after #2669 is done

Oh, yeah, that's probably better. Sorry. 🥲

@Exirel
Copy link
Copy Markdown
Contributor

Exirel commented Feb 22, 2026

@dgw so many conflicts now... 😭

@dgw
Copy link
Copy Markdown
Member Author

dgw commented Feb 22, 2026

Oof, when I get back to this I might reset the branch and start over instead of going through all 40-something conflicts. I bet that would be easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Housekeeping Code cleanup, removal of deprecated stuff, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants