Skip to content

Code Quality Fixes(5) #1523

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
Merged
merged 8 commits into from
Sep 27, 2019
Merged

Code Quality Fixes(5) #1523

merged 8 commits into from
Sep 27, 2019

Conversation

lakshyacis
Copy link
Contributor

@lakshyacis lakshyacis commented Sep 18, 2019 •
edited
Loading

indicator_libmessaging pylint fixes
menu_qrcode pylint fixes
notification_notify2 pylint fixes
plugin pylint fixes
proxyconfig_stem pylint fixes
sound_canberra pylint fixes
sound_gstreamer pylint fixes
sound_playfile pylint fixes

g1itch
g1itch reviewed Sep 18, 2019
View reviewed changes
g1itch
g1itch reviewed Sep 18, 2019
View reviewed changes
@lakshyacis lakshyacis force-pushed the fix#3 branch 4 times, most recently from 19086b4 to 5c847ed Compare September 19, 2019 10:09
g1itch
g1itch requested changes Sep 19, 2019
View reviewed changes
Copy link
Collaborator

@g1itch g1itch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow PEP8

@lakshyacis lakshyacis force-pushed the fix#3 branch 2 times, most recently from dbdbaab to 0b1f6fe Compare September 20, 2019 13:40
PeterSurda
PeterSurda reviewed Sep 21, 2019
View reviewed changes
PeterSurda
PeterSurda requested changes Sep 21, 2019
View reviewed changes
Copy link
Member

@PeterSurda PeterSurda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make the changes requested.

g1itch
g1itch reviewed Sep 21, 2019
View reviewed changes
g1itch
g1itch reviewed Sep 21, 2019
View reviewed changes
g1itch
g1itch reviewed Sep 21, 2019
View reviewed changes
g1itch
g1itch reviewed Sep 21, 2019
View reviewed changes
g1itch
g1itch reviewed Sep 21, 2019
View reviewed changes
g1itch
g1itch reviewed Sep 21, 2019
View reviewed changes
g1itch
g1itch reviewed Sep 21, 2019
View reviewed changes
@lakshyacis lakshyacis force-pushed the fix#3 branch 2 times, most recently from cf07a0e to 8590459 Compare September 26, 2019 13:15
PeterSurda
PeterSurda approved these changes Sep 27, 2019
View reviewed changes
Copy link
Member

@PeterSurda PeterSurda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok

@PeterSurda
Copy link
Member

@g1itch any remaining objections?

@g1itch
Copy link
Collaborator

g1itch commented Sep 27, 2019

@g1itch any remaining objections?

I did not see the abstract-method and logging-not-lazy warnings when created this modules. Also I'm going to remove any extra spaces when I'll edit them again. They doesn't improve readability, just stretch the code.

I don't think this changes improve code quality - this is just formatting and documenting stuff.

@lakshyacis
Copy link
Contributor Author

@g1itch any remaining objections?

I did not see the abstract-method and logging-not-lazy warnings when created this modules. Also I'm going to remove any extra spaces when I'll edit them again. They doesn't improve readability, just stretch the code.

I don't think this changes improve code quality - this is just formatting and documenting stuff.

Pylint show's abstract-method and logging-not-lazy warnings every time when I check. Abstract-method warning is shown because "Method 'save' is abstract in class 'BaseImage' but is not overridden". Please check @PeterSurda on your machine when you free. And @g1itch if you want any changes, please let me know I'll fix.

PeterSurda
PeterSurda reviewed Sep 27, 2019
View reviewed changes
PeterSurda
PeterSurda requested changes Sep 27, 2019
View reviewed changes
Copy link
Member

@PeterSurda PeterSurda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactor log.

@PeterSurda
Copy link
Member

@g1itch

I also see the abstract-method warning when running pylint. I don't care about spaces, other than PEP8:

Inline comments should be separated by at least two spaces from the statement. They should start with a # and a single space.

@PeterSurda PeterSurda self-requested a review September 27, 2019 11:00
@PeterSurda
Copy link
Member

@g1itch Ok now?

@PeterSurda PeterSurda merged commit 433cb98 into Bitmessage:v0.6 Sep 27, 2019
@lakshyacis lakshyacis changed the title Code Quality Fixes Code Quality Fixes(5) Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PeterSurda PeterSurda PeterSurda approved these changes

@g1itch g1itch g1itch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lakshyacis @PeterSurda @g1itch