Skip to content

Conversation

@Exirel
Copy link
Contributor

@Exirel Exirel commented Dec 13, 2020

Description

Working on #1565, for the coretasks core plugin. I wanted to do a quick first draft for @dgw to review.

I'm not really proud of these docstrings. On the other hand, I think we need a full chapter to explain how Sopel initialize the connection, deal with SASL, WHO/WHOX/NAMES, and stuffs. So it's not really my biggest concern.

My biggest concern is the lack of test for this plugin. I might add them here, maybe. We'll see.

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 this to the 7.1.0 milestone Dec 13, 2020
@Exirel Exirel requested a review from dgw December 13, 2020 23:58
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.

OK, this ended up a bit scarier in terms of line-note count than I expected based on the diff size. It's pretty much all trivial stuff, though. I like!

@Exirel
Copy link
Contributor Author

Exirel commented Dec 14, 2020

It's pretty much all trivial stuff, though. I like!

Maybe you didn't notice but I tried hard to not forget about S everywhere. Also, I think I more or less remembered that "plurals' something" and not "plurals's something".

@Exirel Exirel force-pushed the coretasks-docstrings branch from cd43d1e to 81cf6d6 Compare January 14, 2021 16:22
@Exirel
Copy link
Contributor Author

Exirel commented Jan 14, 2021

Fixed conflict and repushed.

@dgw
Copy link
Member

dgw commented Jan 15, 2021

I'm going to trust that Exi's rebase didn't break anything. Did skim the current changes, but not in-depth. (Who wants to re-review things? Not this dgw!)

@dgw dgw merged commit 57d2772 into sopel-irc:master Jan 15, 2021
@Exirel Exirel deleted the coretasks-docstrings branch May 1, 2021 17:54
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.

2 participants