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.

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

tools: move deprecated function to prevent circular import #2206

Closed
Exirel opened this issue Nov 1, 2021 · 4 comments · Fixed by #2232
Closed

tools: move deprecated function to prevent circular import #2206

Exirel opened this issue Nov 1, 2021 · 4 comments · Fixed by #2232
Labels
Milestone

Comments

@Exirel
Copy link
Contributor

Exirel commented Nov 1, 2021

The problem

In a recent PR (#2205), dgw had to move imports around to prevent circular import. This is annoying, because this scenario might repeat. It would be nice to not have circular import, while keeping sopel.tools.deprecated for backward compatibility.

The solution

Edit: we ended with sopel.lifecycle and sopel.tools is backward compatible.

Over time, I feel like sopel.tools should contains extra features and functional utilities, while deprecated is a technical utility function. What I mean is that time functions or web functions are tools for a plugin developers to implement user-facing features. However, deprecated is a technical utility, i.e. something that is useful for developers only (i.e. it tells another developer that this code is deprecated).

I came to the conclusion that we should probably have a sopel.utils that contains non-IRC user features, and more developer oriented features.

One may say that, actually, we already have sopel.formatting that is for IRC users, and sopel.tests that is for developer users outside of sopel.tools. Well, if you ask me, a lot of what is in sopel.tools could go elsewhere too, like jobs could go into sopel.jobs and merge sopel.tools.jobs and sopel.plugins.jobs in one module but... that's probably another issue, right?

Alternatives

I also considered having sopel.tools.utils, that would also fix the initial problem (prevent circular dependencies), however I cannot not overthink everything and I had to push the idea further.

Notes

This should lead to a change in documentation, and probably a removal of backward compatibility in Sopel 9.

@dgw
Copy link
Member

dgw commented Jan 13, 2023

I didn't see this after recent work that touched some things in sopel.tools because it's not milestoned. But #2385 was possibly related to this, as was the earlier patch #2232 to move deprecated into a different submodule.

@Exirel
Copy link
Contributor Author

Exirel commented Jan 13, 2023

Indeed #2232 should close this issue.

@Exirel Exirel linked a pull request Jan 13, 2023 that will close this issue
4 tasks
@Exirel
Copy link
Contributor Author

Exirel commented Jan 13, 2023

After re-reading #2232, it totally closes this one. I don't know why it wasn't in the milestone, or referenced by the PR. I guess mistakes happen sometimes.

@Exirel Exirel closed this as completed Jan 13, 2023
@Exirel Exirel added this to the 8.0.0 milestone Jan 13, 2023
@dgw
Copy link
Member

dgw commented Jan 13, 2023

I guess mistakes happen sometimes.

We're all allowed to forget things every now and then, even 🇫🇷 👨 💻 programmers 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants