{Misc.} Improve code style and prepare for pylint 2.17 #26685
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
Collaborator
|
Misc. |
bebound
commented
Jun 15, 2023
| function-name-hint=lower_case_with_underscores | ||
| argument-name-hint=lower_case_with_underscores | ||
| variable-name-hint=lower_case_with_underscores | ||
| inlinevar-name-hint=lower_case_with_underscores (short is OK) |
Contributor
Author
There was a problem hiding this comment.
xxx-hint is shown when pylint found invalid name:
demo/demo.py:19:0: C0103: Class name "aa" doesn't conform to '{invalid-classname-hint}' pattern ('[_]{0,2}[A-Z]{1}[A-Za-z0-9]{0,30}$' pattern) (invalid-name) (pylint-dev/pylint#2663)
I remove these configs:
- These
xxx-hintconfigs never take effect, asinvalid-nameis disabled since first commit: Touch up pylintrc #3806. This is also mentioned in the comment:
The invalid-name checker must be enabled for these hints to be used.
- The default value is enough, for example,
module-naming-styleissnake_case.
PS:
variable-name-hint raises error from pylint 2.14: pylint-dev/pylint#6931
These options changes to xxxx-style, see https://pylint.pycqa.org/en/latest/user_guide/configuration/all-options.html#basic-checker.
Member
There was a problem hiding this comment.
This is very nice investigation and makes the context much clearer.
bebound
changed the title
bebound
commented
Jun 27, 2023
| def _parse_pair(pair, delimiter): | ||
| if delimiter not in pair: | ||
| InvalidArgumentValueError("invalid format of pair {0}".format(pair)) | ||
| raise InvalidArgumentValueError("invalid format of pair {0}".format(pair)) |
Contributor
Author
There was a problem hiding this comment.
This line makes no sense without raise.
W0133: Exception statement has no effect (pointless-exception-statement)
bebound
commented
Jun 27, 2023
| if kwargs[self._request_param['name']] is None: | ||
| raise ValueError(message.format(self._request_param['model'])) | ||
| if kwargs[self._request_param['name']] is None: | ||
| raise ValueError(message.format(self._request_param['model'])) |
Contributor
Author
There was a problem hiding this comment.
R1720: Unnecessary "else" after "raise", remove the "else" and de-indent the code inside it (no-else-raise)
bebound
commented
Jun 27, 2023
| cmd.cli_ctx, "acrpull", assignee, scope=registry_id, is_service_principal=is_service_principal | ||
| ): | ||
| raise AzCLIError("Could not delete role assignments for ACR. " "Are you an Owner on this subscription?") | ||
| raise AzCLIError("Could not delete role assignments for ACR. Are you an Owner on this subscription?") |
Contributor
Author
There was a problem hiding this comment.
W1404: Implicit string concatenation found in call (implicit-str-concat)
bebound
commented
Jun 27, 2023
|
|
||
| def __format_count(self): | ||
| untouched_keys = [x for x in self.__store.keys() if x not in self.__count.keys()] | ||
| untouched_keys = [x for x in self.__store if x not in self.__count] |
Contributor
Author
There was a problem hiding this comment.
C0201: Consider iterating the dictionary directly instead of calling .keys() (consider-iterating-dictionary)
bebound
commented
Jun 27, 2023
| configs = _generic_site_operation(cmd.cli_ctx, resource_group_name, name, 'get_configuration', slot) | ||
| access_rules = configs.scm_ip_security_restrictions if scm_site else configs.ip_security_restrictions | ||
| ip_exists = [(lambda x: x.ip_address == namespace.ip_address)(x) for x in access_rules] | ||
| ip_exists = [x.ip_address == namespace.ip_address for x in access_rules] |
Contributor
Author
There was a problem hiding this comment.
C3002: Lambda expression called directly. Execute the expression inline instead. (unnecessary-direct-lambda-call)
bebound
commented
Jun 27, 2023
| for edition_idx, edition in enumerate(capability.supported_flexible_server_editions): | ||
| if edition.name == 'MemoryOptimized': | ||
| result[capability_idx].supported_flexible_server_editions[edition_idx].name = 'BusinessCritical' | ||
| capability.supported_flexible_server_editions[edition_idx].name = 'BusinessCritical' |
Contributor
Author
There was a problem hiding this comment.
R1736: Unnecessary list index lookup, use 'capability' instead (unnecessary-list-index-lookup)
bebound
commented
Jun 27, 2023
| def exists(client, table_name): | ||
| generator = client.query_tables("TableName eq '{}'".format(table_name)) | ||
| return list(next(generator.by_page())) != [] | ||
| return bool(list(next(generator.by_page()))) |
Contributor
Author
There was a problem hiding this comment.
C1803: 'list(...) != []' can be simplified to 'list(...)' as an empty list is falsey (use-implicit-booleaness-not-comparison)
bebound
commented
Jun 27, 2023
| # Get from the config file | ||
| return None | ||
| if sum([1 if v is not None else 0 for v in (min_count, max_count)]) == 1: | ||
| if sum(1 if v is not None else 0 for v in (min_count, max_count)) == 1: |
Contributor
Author
There was a problem hiding this comment.
R1728: Consider using a generator instead 'sum(1 if v is not None else 0 for v in (min_count, max_count))' (consider-using-generator)
bebound
changed the title
bebound
force-pushed
the
bump-azdev
branch
2 times, most recently
from
bbe1967 to
da25845
Compare
bebound
commented
Jul 10, 2023
| . env/bin/activate | ||
| python -m pip install -U pip | ||
| pip install azdev | ||
| pip install git+https://github.com/dciborow/azure-cli-dev-tools.git@dciborow/pylint |
Contributor
Author
There was a problem hiding this comment.
This line needs to be removed when azdev is released.
bebound
requested review from
DaeunYim,
andyliuliming,
calvinhzy,
evelyn-ys,
necusjz,
yanzhudd and
zhoxing-ms
as code owners
Member
|
Really detailed explanation for every change! Nice work! @bebound |
wangzelin007
previously approved these changes
Jul 12, 2023
Member
|
@bebound, you have done an outstanding job with great attention to detail. |
bebound
changed the title
evelyn-ys
approved these changes
Jul 17, 2023
wangzelin007
approved these changes
Jul 17, 2023
necusjz
approved these changes
Jul 17, 2023
yanzhudd
approved these changes
Jul 17, 2023
jiasli
approved these changes
Jul 17, 2023
kairu-ms
approved these changes
Jul 17, 2023
avgale
pushed a commit
to avgale/azure-cli
that referenced
this pull request
Aug 24, 2023
This was referenced Nov 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Ensure that the CI passes in the new azdev(Azure/azure-cli-dev-tools#359) and prepare for Python 3.11 support.
Azure is compatible with flake8 6.0 in #25370
Disabled rules:
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE:
az command a: Make some customer-facing breaking change[Component Name 2]
az command b: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.