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

docs: add example of plugin re-use #2489

Merged
merged 1 commit into from
Jul 16, 2023

Conversation

SnoopJ
Copy link
Contributor

@SnoopJ SnoopJ commented Jul 12, 2023

Description

This changeset adds an example to the "tips and tricks" for plugins of importing another plugin and re-using a command from therein, as well as a warning about the potential for the duplication problem described in #2488.

Checklist

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

@dgw dgw added this to the 8.0.0 milestone Jul 12, 2023
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.

These are just style nits. Bigger thing is that you can import a plugin's module even if it's not loaded, so the example could also be used to "rename" a preexisting command.

docs/source/plugin/advanced.rst Show resolved Hide resolved
docs/source/plugin/advanced.rst Outdated Show resolved Hide resolved
Copy link
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'd approve this, however I think there is at least one note to be added: we should say that the single-file plugin type may not work this way, because it may not be importable. You can link to single-file plugin definition with:

Link to :term:`Single file plugin` definition.

docs/source/plugin/advanced.rst Outdated Show resolved Hide resolved
@SnoopJ
Copy link
Contributor Author

SnoopJ commented Jul 14, 2023

we should say that the single-file plugin type may not work this way, because it may not be importable

I'd love to see information of this sort added to Sopel, but I think it's a deeper subject than the warning I wanted to add in this case. I figure that users who are importing not-a-builtin-plugin are probably going to find out about the distinction anyway, and I'm not really sure how to expand this section to tackler the larger subject of how one might go about locating and importing plugins.

Do you think it makes sense to file a separate issue for fleshing out the subject?

@Exirel
Copy link
Contributor

Exirel commented Jul 14, 2023

It's an advanced tips & tricks section, we can direct the reader to more resources without having to explain everything. Beside, there is an explanation about single-file plugin already. I don't think it's that deeper that it requires another issue: a single line in this section is enough in my opinion.

Copy link
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.

🚢

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.

:shipit: (but squash first, please)

@SnoopJ SnoopJ force-pushed the doc/gh2488-warn-about-plugin-unboxing branch from 8ad1244 to 1cb9654 Compare July 16, 2023 19:51
@dgw dgw merged commit a620c14 into sopel-irc:master Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants