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.

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

PEP8 - WIP #225

Closed
wants to merge 4 commits into from
Closed

PEP8 - WIP #225

wants to merge 4 commits into from

Conversation

trenton42
Copy link
Contributor

This is a work in progress, but I wanted to make sure nothing in here was going to be a problem before progressing through other things. Let me know if you have any feedback!

@coveralls
Copy link

coveralls commented Feb 23, 2018

Coverage Status

Coverage remained the same at 95.287% when pulling 79b2b0d on trenton42:pep8 into 85a7ac7 on twisted:master.

@IlyaSkriblovsky
Copy link
Contributor

Why did you added else in line 230, but deleted it in line 292? :) I don't mind, just curious what variant is considered to be a Good Practice™? I personally like else for its explicitness, but don't like for its redundancy, so I'm okay with both.

@psi29a
Copy link
Contributor

psi29a commented Feb 24, 2018

Honestly, I don't care for the 80 column line limit. Can we at least set this 120?

:)

@IlyaSkriblovsky
Copy link
Contributor

Honestly, I don't care for the 80 column line limit. Can we at least set this 120?

yes-yes-yes

@trenton42
Copy link
Contributor Author

Honestly, I don't care for the 80 column line limit. Can we at least set this 120?

I 100% agree as well. If I did trim to 80, it was on accident. I have the 80 column line limit turned off in my PEP8 extension, and instead use the 120 limit from pylint.

@trenton42
Copy link
Contributor Author

@IlyaSkriblovsky Hmm, it looks like I removed it from both 🤔

This is from pylint's R1705:

Unnecessary “else” after “return” Used in order to highlight an unnecessary block of code following an if containing a return statement. As such, it will warn when it encounters an else following a chain of ifs, all of them containing a return statement.

I tend to prefer it, but if you think it goes too far, I can revert.

@IlyaSkriblovsky
Copy link
Contributor

@trenton42, ouch, sorry, you're right

Trenton Broughton added 2 commits February 26, 2018 08:54
msg = "TxMongo: collection names must not start or end with '.', '{0}'".format(repr(name))
msg = "TxMongo: collection names must not start or end with '.', '{0}'".format(
repr(name)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this fit on one line with 120?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think I might have gotten over aggressive on those. I will fix them.

@trenton42 trenton42 closed this by deleting the head repository Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants