Skip to content

Conversation

@Exirel
Copy link
Contributor

@Exirel Exirel commented Dec 21, 2021

Description

More than the title:

  • moved "make it talk", "style", "channels", and "time" in their own file
  • talk, style, and channels are in the same sub-folder, time is at the same level as bot and privileges as it made more sense to me
  • I fixed a warning that appears in the doc when using the Furo theme about "content table"
  • I edited some example, it might be difficult to see them at first, they are in bot/talk.rst (dgw you need to check that, the text is short enough so it shouldn't be too painful)
  • I also added a "best practices" section to the "about time" chapter, since we talked about it on IRC and it made sense to add it in a doc-only PR

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 quality and make test)
  • I have tested the functionality of the things this change touches

@Exirel Exirel added Documentation Housekeeping Code cleanup, removal of deprecated stuff, etc. labels Dec 21, 2021
@Exirel Exirel added this to the 8.0.0 milestone Dec 21, 2021
@Exirel Exirel requested a review from dgw December 21, 2021 22:55
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.

Just one line note from me this time. I don't think there's much else for me to say yet. Excluding the already-merged "time" page, this PR's net line diff is only about +10, I've explicitly not looked at the moved page contents in any detail because I trust (from skimming) that they're nearly identical.

@Exirel Exirel force-pushed the docs-plugin-bot-split branch from 93c7abf to d1a50ab Compare December 23, 2021 13:29
@Exirel Exirel marked this pull request as ready for review December 23, 2021 13:29
@Exirel
Copy link
Contributor Author

Exirel commented Dec 23, 2021

I rebased on master so you don't see the other PR's commit in here anymore. I did so after applying your suggestion, which I kept in a separate commit. Nothing else has change since your last review.

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.

Here I am with more thoughts. This is certainly not the end of bikeshedding. 🙀

@Exirel Exirel requested a review from dgw January 8, 2022 16:03
@Exirel
Copy link
Contributor Author

Exirel commented Jan 8, 2022

@dgw I changed the wording for join/part, and I'm now tempted to write a guideline about time (the thing we discussed on IRC when playing with timeformat) in that PR, if that's cool with you.

@Exirel
Copy link
Contributor Author

Exirel commented Jan 8, 2022

Now with Best Pratices for times:

image

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.

In other news, I'm thinking (again) about a :pypi: custom Sphinx role…

@Exirel
Copy link
Contributor Author

Exirel commented Jan 26, 2022

Like I said on IRC, I realized that the conflict is more annoying than risky. However, given it's a doc PR, I don't want to rebase & squash before @dgw agree to, otherwise he'll complain about tracking changes. 😁

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 saw one more nit, but you can apply it (or not) during the squash. I'm not going to spend the time post-squash checking to make sure you actually used it. 😛

@Exirel Exirel force-pushed the docs-plugin-bot-split branch from 7407d35 to 30d7015 Compare February 17, 2022 22:40
@Exirel
Copy link
Contributor Author

Exirel commented Feb 17, 2022

Applied, squashed & done.

@Exirel Exirel mentioned this pull request Feb 18, 2022
4 tasks
@dgw dgw merged commit f595114 into sopel-irc:master Feb 20, 2022
@Exirel Exirel deleted the docs-plugin-bot-split branch February 23, 2022 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation Housekeeping Code cleanup, removal of deprecated stuff, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants