-
-
Notifications
You must be signed in to change notification settings - Fork 409
docs: the plugin chapter #1923
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: the plugin chapter #1923
Conversation
118524f to
222ed1a
Compare
|
@dgw if by chance you have the time to review some documentation, I think I'm more or less done with:
That last one in particular: I took the same approach I had with the configuration, i.e. "Anatomy of a plugin" is an exploration of everything around a plugin and its rules (which is at the core of any plugin), and I left the autodoc part in another section. I really like how it turned out, but you know how poor my English can be. The next steps in that documentation PR are probably about how to use the |
dgw
left a comment
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.
I did indeed find time to review this, somehow, despite getting almost no time to myself all day. Too bad for you, though—there are a lot of line notes! 😝
|
Whipped through here again right before my next meeting. Hopefully I answered all the little sub-threads that needed it, so you can proceed with your next steps. 😸 |
5b60cc1 to
3d5e5eb
Compare
|
Added some documentation about the |
9221397 to
c5f41ca
Compare
|
@dgw brace yourself, it's the real one now. I haven't added that many things, and yet, I'm sure you'll find plenty to fix. |
dgw
left a comment
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.
Did not expect this many line notes in another go-around, but here we are. Have fun! 😅
I'm also thinking about the URL structure, but it's not clear to me that /docs/plugin/ (or /docs/plugin/index.html) would be meaningfully better than /docs/plugin.html. Am also thinking about whether to make it /docs/plugin/* (current PR) or /docs/plugins/*… The singular matches the Sopel submodule that plugin devs work with the most, I guess.
docs/source/plugin/what.rst
Outdated
| * you don't have any dependencies other than Sopel and the Python's built-in | ||
| library |
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.
New alternative suggestion, since we haven't gotten around to having this conversation yet… (sadly)
| * you don't have any dependencies other than Sopel and the Python's built-in | |
| library | |
| * you don't have any dependencies other than Sopel and Python's built-in library, | |
| or your plugin is a "one-off" for your use only (not intended to be shared) |
I still don't see why we'd mention dependencies for this, but not the Folder type.
I added a whole new section, that checks out.
Both work for me, so your call! It's one |
|
@Exirel I've seen the fixes since my last review, finally. Go ahead and squash/rebase, then I'll re-review one more time before hopefully merging. There can't be many typos left by now! 😅 |
|
<loud happy noise> |
686b645 to
ddb5494
Compare
|
Done! |
|
@dgw I've added a new commit with some minor changes, to address the dependencies question. I added that it is better not to have dependencies, because it is more difficult to share the plugin. I added a note to tell plugin authors to write the list of dependencies in a I still think Folder plugins are not a good idea, as you need to add a directory to the |
dgw
left a comment
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.
I'm doing my best to make good suggestions for these new notes now, so you can incorporate them (or get inspired and make your own changes) before going to bed tonight in CEST. 😸 Then I have a prayer of doing that full re-review while you sleep. 😁
|
@dgw if I didn't miss anything, we should be good now! |
dgw
left a comment
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.
I think we can finish this over the weekend! Home stretch! Glad I did one last full re-review, though. Caught a few stupid typos that I missed the first n times around. x)
|
@dgw I'm ready to rebase & squash asap. |
dgw
left a comment
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.
Go for it! 🐑
Initial version by @Exirel and grammar/spelling fixed by @dgw Co-authored-by: dgw <[email protected]>
Thank you dgw for your dedication and hard work. Co-authored-by: dgw <[email protected]>
Co-authored-by: dgw <[email protected]>
a9d5a8f to
5a2fdf5
Compare
|
After ~150 comments and many reviews, I've rebased & squashed the whole thing. I'm quite proud of this work, and I hope this will open the path for even greater documentation in the future. Ready to merge! |
Description
I moved the content of
source/plugin.rsttosource/plugin/*.rstfiles and then worked around that structure:botto talk, join channels, and stuffs)sopel.plugin(plugin decorators, already explained in anatomy of a plugin)sopel.plugins(internals, no need to touch that)I took the same approach I used for the core configuration: one part is a thorough explanation of the topic and the other part is the good old
autodoc. I really like it!I think it's possible to do even more, and that should be done later, either by me or someone else, in this version or a future one. The most important here is that I think it's a good structure to work with.
Checklist
make qa(runsmake qualityandmake test)