sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
950 stars 405 forks source link

modules: rename built-in plugins to "builtins" #2504

Closed dgw closed 8 months ago

dgw commented 11 months ago

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 under sopel.externals.*), but I liked builtins better than internals.

Updated relevant documentation examples and test code.

Have not renamed any functions/methods/attributes/variables in code that manipulates plugins from the sopel.modules (now sopel.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

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.

Exirel commented 11 months ago

Or can we just call this a non-breaking change because the core plugins aren't part of our public API?

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.

Exirel commented 8 months ago

Just a quick note to say that I fully approve of this PR btw.

dgw commented 8 months ago

Rebased on master to resolve merge conflicts and tweaked the find_internal_plugins() code after all.

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.