{Pylint} Fix consider-using-in #30349
Draft
Conversation
This file contains 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
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
This is a big task: |
12 tasks
|
|
jiasli
commented
Nov 15, 2024
| @@ -51,7 +51,7 @@ def validate_import_key(key): | |||
| if not isinstance(key, str): | |||
| logger.warning("Ignoring invalid key '%s'. Key must be a string.", key) | |||
| return False | |||
| if key == '.' or key == '..' or '%' in key: | |||
| if key in (".", "..") or "%" in key: | |||
There was a problem hiding this comment.
I am not sure why ruff changes ' to ".
jiasli
commented
Nov 15, 2024
| @@ -482,7 +482,7 @@ def _validate_ase_is_v3(ase): | |||
|
|
|||
|
|
|||
| def _validate_ase_not_ilb(ase): | |||
| if (ase.internal_load_balancing_mode != 0) and (ase.internal_load_balancing_mode != "None"): | |||
There was a problem hiding this comment.
() are unnecessary as != has higher precedence: https://docs.python.org/3/reference/expressions.html#operator-precedence
jiasli
commented
Nov 15, 2024
Comment on lines
+361
to
+362
| if not (ns.public_access in ('Disabled', 'Enabled') or | ||
| val in ('all', 'none') or (len(val.split('-')) == 1 and _validate_ip(val)) or |
There was a problem hiding this comment.
The outer () of (len(val.split('-')) == 1 and _validate_ip(val)) are unnecessary. ruff automatically fixes it:
if not (ns.public_access in ('Disabled', 'Enabled') or val in ('all', 'none') or len(val.split('-')) == 1 and _validate_ip(val) or len(val.split('-')) == 2 and _validate_ip(val)):but I am not going to fix it here.
jiasli
commented
Nov 15, 2024
| @@ -442,8 +442,7 @@ def _get_service_principal_object_from_type(servicePrincipalType): | |||
| servicePrincipalResult = None | |||
|
|
|||
| if (servicePrincipalType is not None and | |||
| (servicePrincipalType == ServicePrincipalType.system_assigned.value or | |||
| servicePrincipalType == ServicePrincipalType.none.value)): | |||
| (servicePrincipalType in (ServicePrincipalType.system_assigned.value, ServicePrincipalType.none.value))): | |||
There was a problem hiding this comment.
This breaks flake8:
Running flake8 on modules...
ERROR: /mnt/vss/_work/1/s/src/azure-cli/azure/cli/command_modules/sql/custom.py:445:9: E129 visually indented line with same indent as next logical line
/mnt/vss/_work/1/s/src/azure-cli/azure/cli/command_modules/sql/custom.py:445:9: E129 visually indented line with same indent as next logical line
2 E129 visually indented line with same indent as next logical line
See https://stackoverflow.com/questions/44638211/pep8-contradiction-between-e129-and-e127-e128
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Finish the unfinished tasks of #20192
Fix
consider-using-in: https://pylint.readthedocs.io/en/latest/user_guide/messages/refactor/consider-using-in.htmlAuto-fixed with
ruff:consider-using-in/R1714is implemented asPLR1714inruff: