-
-
Notifications
You must be signed in to change notification settings - Fork 403
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.
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
modules: rename built-in plugins to "builtins" #2504
Conversation
I think it's a breaking change if we really want to prevent breaking someone's heating system. Otherwise, I say we go for it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, passes QA for me locally.
+1 to making it a breaking change. No sense in drawing out the removal of the old name even longer.
Just a quick note to say that I fully approve of this PR btw. |
Updated relevant plugin handling, documentation/examples, & test files.
Rebased on Will most likely merge this next because it touches so damned many files; other open PRs can be manually merged (if approved) or rebased as necessary. This is just one of those situations where "the perfect merge opportunity" will never come. Big PRs cause conflicts, but it doesn't make sense to do this kind of thing piecemeal. |
Description
Finally does away with deadnaming core plugins as "modules". The alternative we were thinking of a while ago was
sopel.internals
(to mirror how custom plugins not part of a Python package emit logs undersopel.externals.*
), but I likedbuiltins
better thaninternals
.Updated relevant documentation examples and test code.
Have not renamed any functions/methods/attributes/variables in code that manipulates plugins from the
sopel.modules
(nowsopel.builtins
) module namespace.Opening as draft first, since I made most of these modifications in a terminal emulator on my phone and have definitely not had a chance to double check everything yet. (Also the reason that half the checklist below is blank for now.)
Milestoned for 8.0.0 to start, but I'll be the first one to bump it into 8.1.0 if everything else is done but this isn't ready yet for any reason.
Checklist
make qa
(runsmake lint
andmake test
)Open question
Do we need to keep shims in
sopel.modules
in case custom plugins import from those, and set a timeline for removing them for real later (in 9.0, probably)? Or can we just call this a non-breaking change because the core plugins aren't part of our public API?I know I'm in favor of just moving the files, without any shimming or deprecation cycle, but I'm curious if anyone disagrees with that position.