-
-
Notifications
You must be signed in to change notification settings - Fork 106
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
More pylint fixes #133
More pylint fixes #133
Conversation
Removed unnecessary ... elipsis after comment / doc blocks Removed fstring when no interpolation of variables
Merging from miksus base
Trailing whitespace and more
typo
Thanks! I'll try to check and merge before the end of the week. Sorry for the delay. |
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.
Just some (perhaps opinionated) suggestions to leave the "..." in some code snippets in the documentation to indicate a placeholder and there should be more code in practice.
For example:
def do_things():
"This does things"
...
Vs:
def do_things():
"This does things"
Sorry it took almost a month to review (had a bad week, a busy week and a small side project). I think all of those changes are appropriate but I think some of those placehoder That contribution.md is also a nice addition. Perhaps will motivate others to do an excellent work for the library just as you have done :) Though I would emphasise your fixes role more as you have significantly cleaned up the code base. |
reintroduce ellipsis
reintroduce ellipsis
reintroduce ellipsis
reintroduce ellipsis
Codecov ReportBase: 94.88% // Head: 94.90% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #133 +/- ##
==========================================
+ Coverage 94.88% 94.90% +0.02%
==========================================
Files 80 80
Lines 4454 4454
==========================================
+ Hits 4226 4227 +1
+ Misses 228 227 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
reintroduce ellipsis
added more detail
reintroduce ellipsis
reintroduce ellipsis
reintroduce ellipsis
reinstated ellipsis
reinstated ellipsis
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.
Think I've made them all
Still says changes requested but I think I've resolved all? Just pulling in latest from your repo to see if I've missed any new ones |
trailing whitespace issues
Yep, looks good. Thanks again for the fixes and sorry for the immense delay. I now booked an hour every week to go through PRs and issues so hopefully, there won't be the same delay anymore. |
All good. I'm still learning so in no hurry. |
Including some newly introduced ones when I rebased