-
Notifications
You must be signed in to change notification settings - Fork 63
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
Additional Pep8speaks updates #856
Conversation
Hello @BradleySappington, Thank you for updating ! There are no PEP8 issues in this Pull Request. Comment last updated at 2024-05-22 20:20:33 UTC |
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.
Hi @BradleySappington thanks for these edits. Most looks fine but I have a couple concerns, particularly about line breaks in strings.
I think using a backslash to break a string across two lines doesn't work in the way you're intending it to here: the whitespace at the start of the second line isn't trimmed out automatically. I tested several of those edits, and the proposed edits do result in extra unintended whitespace in the output strings, so I think those will need to be revised. There are quite a few instances of this, as marked below (I may not have caught them all).
This is a small thing but I don't think we want to accidentally introduce formatting errors in output messages in the course of cleaning up formatting errors of the code 😆
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.
Thanks again for pointing out the formatting mistake!
@mperrin - updates complete |
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.
Looks good. Thanks @BradleySappington
Found a few missed items from the no-formatting transition