-
-
Notifications
You must be signed in to change notification settings - Fork 669
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
Run docformatter==v1.6.1
#1886
Run docformatter==v1.6.1
#1886
Conversation
If you don't want to encourage PRs like this just to review a bunch of docstrings, feel free to close this :) |
For more information on this file, see | ||
https://docs.djangoproject.com/en/4.1/topics/settings/ | ||
For more information on this file, see: | ||
- https://docs.djangoproject.com/en/4.1/topics/settings/ | ||
|
||
For the full list of settings and their values, see | ||
https://docs.djangoproject.com/en/4.1/ref/settings/ | ||
For the full list of settings and their values, see: | ||
- https://docs.djangoproject.com/en/4.1/ref/settings/ |
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.
docformatter
really wanted to insert a bunch of newlines here so i added some symbols to try to stop that from happening.
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.
Interesting that it doesn't add ticks around the URL like it did in the briefcase patch
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.
Oh....i actually did that manually in the briefcase update. I had forgotten I did that.
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've made one very minor tweak for consistency, but otherwise this all looks good. I guess a periodic manual pass of docformatter will help catch the most common issues, and we can manually fix any ReST issues we find.
core/src/toga/widgets/base.py
Outdated
widget?""" | ||
"""Is the widget currently enabled? | ||
|
||
i.e., can the user interact with the widget? |
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.
Breaking this sentence up doesn't make much sense; not sure why it's been broken here, but not in other identical sentences on other widgets.
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.
The first line of a docstring should be one complete sentence. Subsequent sentences will be separated by an empty line....as least AFAIK....
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.
Sure - that's my understanding as well... but it didn't do this on Box (and a couple of others). It's the inconsistency that is weirding me out.
@@ -29,17 +29,17 @@ def __init__( | |||
|
|||
@property | |||
def enabled(self): | |||
"""Is the widget currently enabled? i.e., can the user interact with the | |||
widget? | |||
"""Is the widget currently enabled? i.e., can the user interact with the widget? |
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 not clear why this didn't get split into 2 lines where the implementation on base did.
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.
hmm....my guess is because it has an empty line and more text following. So, I guess the "one sentence on first line" rule isn't being enforced here....
For more information on this file, see | ||
https://docs.djangoproject.com/en/4.1/topics/settings/ | ||
For more information on this file, see: | ||
- https://docs.djangoproject.com/en/4.1/topics/settings/ | ||
|
||
For the full list of settings and their values, see | ||
https://docs.djangoproject.com/en/4.1/ref/settings/ | ||
For the full list of settings and their values, see: | ||
- https://docs.djangoproject.com/en/4.1/ref/settings/ |
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.
Interesting that it doesn't add ticks around the URL like it did in the briefcase patch
Changes
docformatter
v1.6.1 over the entire code base manually coaxing a few spots to play alongdocformatter
config:Notes
docformatter
in generalPR Checklist: