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.

Sign up for GitHub

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

Jump to bottom

cli: modernize docstrings #1922

Merged
merged 1 commit into from
Sep 13, 2020
Merged

cli: modernize docstrings #1922

merged 1 commit into from
Sep 13, 2020

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Aug 9, 2020 •
edited
Loading

Description

Part of #1565 for sopel.cli.

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 Aug 9, 2020
@Exirel Exirel requested a review from a team August 9, 2020 22:35
dgw
dgw requested changes Aug 13, 2020
View reviewed 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 made some style suggestions to stay in line with what already exists in other modules.

Big comment for everything: First line of docstring should end with a . if we're going to be consistent. Pretty sure that's the style we decided on all those weeks/months ago.

sopel/cli/config.py Outdated Show resolved Hide resolved
sopel/cli/config.py Outdated Show resolved Hide resolved
sopel/cli/config.py Outdated Show resolved Hide resolved
sopel/cli/plugins.py Outdated Show resolved Hide resolved
sopel/cli/plugins.py Outdated Show resolved Hide resolved
sopel/cli/plugins.py Outdated Show resolved Hide resolved
sopel/cli/plugins.py Outdated Show resolved Hide resolved
sopel/cli/run.py Outdated Show resolved Hide resolved
sopel/cli/run.py Outdated Show resolved Hide resolved
sopel/cli/run.py Outdated Show resolved Hide resolved
@dgw dgw mentioned this pull request Aug 13, 2020
Closed
26 tasks
@Exirel
Copy link
Contributor Author

Exirel commented Aug 13, 2020

From the CONTRIBUTING.md file:

A one-line description, as an imperative sentence (ending in a period)

It's on me.

@Exirel Exirel requested a review from dgw August 13, 2020 07:36
dgw
dgw reviewed Aug 13, 2020
View reviewed 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.

One batch of suggestions kinda unrelated to the PR topic, except that those lines are already getting touched. I'll probably re-review from scratch to (try to) make sure I've not missed anything else. 🔍

sopel/cli/utils.py Outdated Show resolved Hide resolved
sopel/cli/utils.py Outdated Show resolved Hide resolved
sopel/cli/utils.py Outdated Show resolved Hide resolved
@Exirel Exirel requested a review from dgw August 15, 2020 13:01
@Exirel
Copy link
Contributor Author

Exirel commented Aug 15, 2020

Rebased & squashed, as it's ready to be merged, unless @dgw finds new mistakes.

dgw
dgw approved these changes Aug 28, 2020
View reviewed 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.

unless @dgw finds new mistakes.

Challenge Accepted™ 😎 …but I didn't find much. Not even worth a "Request changes"; you'll update these if you feel like it between now and when I merge this, or you won't. I don't care that much. :P

sopel/cli/config.py Outdated Show resolved Hide resolved
sopel/cli/config.py Outdated Show resolved Hide resolved
@Exirel @dgw
cli: modernize docstrings
1532f8c 
As usual, spelling & grammar fixed by @dgw

Co-authored-by: dgw <dgw@technobabbl.es>
@Exirel
Copy link
Contributor Author

Exirel commented Aug 28, 2020

:shipit: :shipit: :shipit: :shipit:

@dgw dgw merged commit 58bd737 into sopel-irc:master Sep 13, 2020
@Exirel Exirel deleted the cli-docstrings branch May 1, 2021 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgw dgw dgw approved these changes

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
7.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@Exirel @dgw