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: install and running instance documentation #2533

Merged
merged 2 commits into from
Nov 26, 2023

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Oct 28, 2023

Description

This is my attempt at working on #2497 with a version that I think could be good enough for Sopel 8.0.0.

I keep it as a draft to get feedfback more than review i.e. more about what content should be or shouldn't be in here.

I took a lot from Installing to know what to put in here. I think the next step (here or in a future PR?) would be to add more from the per-channel config page.

What I mean is: what do you think?

Checklist

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

Note: couldn't run make qa because mypy 1.6.1 fails on a line that is already in current master.

@Exirel Exirel added this to the 8.0.0 milestone Oct 28, 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.

Once I got going it was actually hard to stop. "Just one more typo fix!"

docs/source/run.rst Outdated Show resolved Hide resolved
docs/source/run/install.rst Outdated Show resolved Hide resolved
docs/source/run/install.rst Outdated Show resolved Hide resolved
docs/source/run/install.rst Outdated Show resolved Hide resolved
docs/source/run/install.rst Outdated Show resolved Hide resolved
docs/source/run/service.rst Outdated Show resolved Hide resolved
docs/source/run/service.rst Outdated Show resolved Hide resolved
docs/source/run/service.rst Outdated Show resolved Hide resolved
docs/source/run/service.rst Outdated Show resolved Hide resolved
docs/source/run/service.rst Outdated Show resolved Hide resolved
@Exirel
Copy link
Contributor Author

Exirel commented Oct 29, 2023

A note here: yes, I tested the systemd's service unit myself, and it worked like a charm! I even used it with sopel-remind to see if it worked as intended with a plugin that create its own files from the homedir. I've to say that systemd and its automatic creation of folders with the appropriate chown & chmod is very convenient. Maybe in the future I'll consider adding a better integration with systemd, because it defines env vars that are very convenient to reuse!

@SnoopJ
Copy link
Contributor

SnoopJ commented Oct 29, 2023

Note: couldn't run make qa because mypy 1.6.1 fails on a line that is already in current master.

Out of curiosity, what's the failure? I can't reproduce with that mypy version against either this branch or abc9322

@Exirel
Copy link
Contributor Author

Exirel commented Oct 29, 2023

Out of curiosity, what's the failure? I can't reproduce with that mypy version against either this branch or abc9322

$ mypy --check-untyped-defs sopel
sopel/lifecycle.py:147: error: Unsupported operand types for >= ("tuple[int, ...]" and "None")  [operator]
sopel/lifecycle.py:147: note: Right operand is of type "tuple[int, ...] | None"

@Exirel
Copy link
Contributor Author

Exirel commented Oct 29, 2023

We figured out, thanks to @SnoopJ intuition that it was about packaging. So here is #2535 to fix that!

@Exirel Exirel marked this pull request as ready for review November 7, 2023 21:17
@ghost
Copy link

ghost commented Nov 7, 2023

Summary from IRC spam:

also super doesn't matter, but I'd put "running as a service" before "managing plugins" :P -- or even as just a part of the install page tbh

but idk, that could just be me

should push sopel-help instead of sopel-remind in the "managing plugins" page ;P

I would also consider links like https://pypi.org/search/?q=sopel- or https://github.com/orgs/sopel-irc/repositories?type=all being included on the plugins page. Maybe even a "curated" list of "must-haves" :: Might also consider tagging all your plugin repos with like sopel-plugin or something so it can just be a "topic" on GitHub as another method of discovery.

might also add a note somewhere that comments (any line starting with #) you put into the config file gets wiped out :: if that's already there and I missed it...oops?

ah it's on the later [core] page

also...time to remove python2 mentions from the docs, no?

anyway, other than the contrast on some stuff in dark mode (I flipped between light and dark a lot), I think it's very solid

and it looks much nicer than the old docs ;P

@Exirel
Copy link
Contributor Author

Exirel commented Nov 7, 2023

also super doesn't matter, but I'd put "running as a service" before "managing plugins" :P -- or even as just a part of the install page tbh
but idk, that could just be me

I wondered the same thing, and I'm still on the fence. I know that I would rather have "running as a service" as its own page, so people may contribute more easily with a "here is the version for my setup that work 100% I swear".

My initial reasoning for plugin before service was that someone may want to play locally with Sopel first, install a few plugins, and see how it behaves. And then, after tinkering with it, create a proper service for their "production" environment. But that could be just me. Having the service doc first would probably invite people to consider the proper approach first.

I don't know. I'll let someone else decide (@dgw in particular).

@dgw
Copy link
Member

dgw commented Nov 8, 2023

Installing Sopel as a service does make sense as a separate page for all the previously mentioned reasons, but could perhaps be a subpage of the "Install Guide" instead of a sibling.

Might also consider tagging all your plugin repos with like sopel-plugin or something so it can just be a "topic" on GitHub as another method of discovery.

Maybe even a "curated" list of "must-haves"

We have this. The other discovery methods are cool, but no one's really forcing packages or repos to be named a certain way. Plenty of plugins sit around in repos named like <bot_nick>-modules or some other offbeat scheme. Every suggested way we have of finding other Sopel plugins depends on the plugin author naming the package/repo a certain way, or remembering to apply a tag/topic, or submitting it to a curated-list repo (we've had a WIP for ages).

To this day I still frequently look for random plugins using a code search for "thingiwant sopel lang:python" because the most reliable way to tell if a project might be a Sopel plugin is if it imports sopel. 🤷‍♂️

should push sopel-help instead of sopel-remind in the "managing plugins" page ;P

That wouldn't quite fit the tutorial flow, since sopel-help gets auto-installed along with sopel; it's one of the dependencies. sopel-remind is an example plugin that won't be installed by default, so the whole add-a-plugin process can be explored starting from pip install.

anyway, other than the contrast on some stuff in dark mode (I flipped between light and dark a lot), I think it's very solid

and it looks much nicer than the old docs ;P

Please take any issues with the theme design to Furo's repo, where I know the topic has previously come up from time to time. Otherwise, for Sopel-specific issues with contrast, you'll have to specify where to look. 🙂

Copy link
Contributor

@SnoopJ SnoopJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had plenty of things to say, but they are mostly nits. I approve, well done!

docs/source/run/install.rst Outdated Show resolved Hide resolved
docs/source/run.rst Outdated Show resolved Hide resolved
docs/source/run/install.rst Outdated Show resolved Hide resolved
docs/source/run/install.rst Outdated Show resolved Hide resolved
docs/source/run/install.rst Outdated Show resolved Hide resolved
docs/source/run/plugin.rst Outdated Show resolved Hide resolved
docs/source/run/plugin.rst Outdated Show resolved Hide resolved
docs/source/run/plugin.rst Outdated Show resolved Hide resolved
docs/source/run/plugin.rst Outdated Show resolved Hide resolved
docs/source/run/service.rst Outdated Show resolved Hide resolved
docs/source/run/service.rst Show resolved Hide resolved
docs/source/run/service.rst Show resolved Hide resolved
docs/source/run/service.rst Show resolved Hide resolved
@Exirel
Copy link
Contributor Author

Exirel commented Nov 16, 2023

Alright, there is one conflict on configuration.rst that I haven't considered yet. I can rebase to fix that.

However, I think I've fixed most of the nitpick and taken into account most of the feedbacks. There is one conversation unresolved because I'd like a different link while keeping @SnoopJ 's suggestion.

I've removed the single instance systemd service file from contribs/ and updated the related README accordingly. I took the opportunity to add a full example of a default configuration file to connecto to Libera.

I added a bunch of links to libera, github, and created a very small FAQ section that can grow every new release (major, minor, or patch version should be fine for that!). That way there are now several references to "how to contact us" in the documentation, all pointing to the FAQ and "libera.chat, #sopel channel".

@sopel-irc/rockstars have fun reading that (again)!

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.

I caught what I could. I'm confident that the other reviewers will catch most of what I missed. 😁

contrib/README.md Outdated Show resolved Hide resolved
contrib/sopel.cfg Show resolved Hide resolved
docs/source/faq.rst Outdated Show resolved Hide resolved
docs/source/faq.rst Outdated Show resolved Hide resolved
docs/source/faq.rst Outdated Show resolved Hide resolved
docs/source/index.rst Show resolved Hide resolved
docs/source/run.rst Outdated Show resolved Hide resolved
docs/source/run/install.rst Show resolved Hide resolved
docs/source/run/service.rst Outdated Show resolved Hide resolved
docs/source/run/service.rst Outdated Show resolved Hide resolved
@Exirel Exirel force-pushed the docs-install branch 2 times, most recently from 9859eab to 0faf0b8 Compare November 25, 2023 00:30
@Exirel
Copy link
Contributor Author

Exirel commented Nov 25, 2023

Alright folks. It's pretty much ready to go live. This contains a pretty good next step for the "running the bot" chapter, with detailed instructions to install, configure, and run the bot, and there is even a start of a FAQ to iterate on in future PR.

I rebased to get the latest fix and doc's config modifications, and squashed everything because at this point it was just "apply review" commits. I invite you to read the preview (link in the CI's report), it looks pretty darn good to me!

@SnoopJ
Copy link
Contributor

SnoopJ commented Nov 25, 2023

I agree, let's get it in! The best way to get additional improvements here is to get this additional documentation in front of real users 😈

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.

Maybe if @SnoopJ had posted his last comment as a review instead, I wouldn't have bothered going through again. But I did, and I have a few final suggestions. (Before anyone says anything: Sometimes even I hate how nitpicky I am, but it's for a good cause!)

contrib/README.md Show resolved Hide resolved
docs/source/run.rst Outdated Show resolved Hide resolved
docs/source/run/install.rst Outdated Show resolved Hide resolved
docs/source/run/plugin.rst Outdated Show resolved Hide resolved
docs/source/run/plugin.rst Show resolved Hide resolved
docs/source/run/plugin.rst Outdated Show resolved Hide resolved
docs/source/run/plugin.rst Outdated Show resolved Hide resolved
docs/source/run/service.rst Outdated Show resolved Hide resolved
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.

Apparently you liked pretty much all of my nitpicks. As a reward (?) I won't go through again and risk finding more. 😜

* explain how to install Sopel in the doc
* explain how to manage plugins
* explain how to use systemd to run Sopel as a service
* start a short FAQ
* add information to contact the Sopel team on IRC
* reorganize the "run Sopel" chapter
* fix links to config and all that were moved
* remove systemd service files from contribs

Co-authored-by: dgw <[email protected]>
Co-authored-by: James <[email protected]>
@dgw dgw merged commit 7693af3 into sopel-irc:master Nov 26, 2023
13 checks passed
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.

4 participants