chore: Make pylintrc ignore fewer rules and pylintrc more readable #2522
Conversation
aahung
changed the title
There was a problem hiding this comment.
A lot of these changes seem to be about pylint enforcing a draconian line length limit. black on the other hand already partially enforces this, but in a more pragmatic manner ("an inherently long URL is long, so it's okay"). I wonder if it might be better to turn of the length enforcement and delegate that to black?
| @@ -41,7 +41,7 @@ def handle_parse_result(self, ctx, opts, args): | |||
| if mutex_opt in opts: | |||
| req_cnt -= 1 | |||
|
|
|||
| if req_cnt == 0: | |||
| if not req_cnt: | |||
There was a problem hiding this comment.
Does it complain about this? It's weird using boolean logic on integers.
There was a problem hiding this comment.
In pylint's source
Most of the times you should use the fact that integers with a value of 0 are false.
An exception to this rule is when 0 is allowed in the program and has a
different meaning than None!
https://github.com/PyCQA/pylint/blob/master/pylint/extensions/comparetozero.py#L23
samcli/commands/publish/command.py
Outdated
| SAM_PUBLISH_DOC = "https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/serverless-sam-template-publishing-applications.html" # noqa | ||
| SAM_PACKAGE_DOC = "https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/sam-cli-command-reference-sam-package.html" # noqa | ||
| SAM_DOC_PREFIX = "https://docs.aws.amazon.com/serverless-application-model/latest/developerguide" | ||
| SAM_PUBLISH_DOC = f"{SAM_DOC_PREFIX}/serverless-sam-template-publishing-applications.html" # noqa |
There was a problem hiding this comment.
What was the error here? Doesn't noqa prevent the complaints? Feels more of a fix to deal with pylints whims.
There was a problem hiding this comment.
It was a fix. I will remove # noqa here instead of reverting in this case because making the common prefix into a variable does make it easier to maintain
| # Only sleep if there have been no retry_attempts | ||
| time.sleep(self.client_sleep if retry_attempts == 0 else 0) | ||
| time.sleep(0 if retry_attempts else self.client_sleep) |
There was a problem hiding this comment.
What was the complaint here? As above, not sure why it prefers boolean logic on integers.
black does nothing about strings, it won't split long string like this, either make a super long comment line shorter. But I do agree we should make URLs exceptions |
|
I reverted the changes that split URLs into multiple lines. It made copy/paste difficult. For all other splitting-the-line scenarios, I think they are pretty legit and I don't have a good reason not to do them. |
…ws#2522) * chore: Make pylintrc ignore rule more readable * chore: Remove pylint rules that do not apply for python 3 * chore: Enable pylint rule Unused import * chore: Enable pylint multi-processes * chore: Enable pylint line too long * chore: Remove unnecessary pylint disable * chore: Enable pylint Else If Used checker * chore: Enable pylint compare to zero/empty string
Which issue(s) does this change fix?
Why is this change necessary?
This brings back some pylint improvements which were originally implemented in #2375 and reverted in #2400 to make room for a massive merge.
How does it address the issue?
What side effects does this change have?
Checklist
make prpassesmake update-reproducible-reqsif dependencies were changedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.