Sourcery refactored dev branch #1
Open
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
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
| # ----------------------------------------------------------------------------- | ||
|
|
||
| """Azure Developer Tools package that can be installed using setuptools""" | ||
|
|
Author
There was a problem hiding this comment.
Lines 19-20 refactored with the following changes:
- Replace m.group(x) with m[x] for re.Match objects (
use-getitem-for-re-match-groups)
|
|
||
| for scope in ['cli', 'extension']: | ||
| with ArgumentsContext(self, '{} generate-docs'.format(scope)) as c: | ||
| with ArgumentsContext(self, f'{scope} generate-docs') as c: |
Author
There was a problem hiding this comment.
Function load_arguments refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
Comment on lines
-12
to
+21
| if namespace.storage_account: | ||
| if not is_valid_resource_id(namespace.RESOURCE): | ||
| namespace.storage_account = resource_id( | ||
| subscription=get_subscription_id(cmd.cli_ctx), | ||
| resource_group=namespace.resource_group_name, | ||
| namespace='Microsoft.Storage', | ||
| type='storageAccounts', | ||
| name=namespace.storage_account | ||
| ) | ||
| if namespace.storage_account and not is_valid_resource_id( | ||
| namespace.RESOURCE | ||
| ): | ||
| namespace.storage_account = resource_id( | ||
| subscription=get_subscription_id(cmd.cli_ctx), | ||
| resource_group=namespace.resource_group_name, | ||
| namespace='Microsoft.Storage', | ||
| type='storageAccounts', | ||
| name=namespace.storage_account | ||
| ) |
Author
There was a problem hiding this comment.
Function example_name_or_id_validator refactored with the following changes:
- Merge nested if conditions (
merge-nested-ifs)
Comment on lines
-65
to
+67
| raise CLIError('Unable to find `{}` repo. Have you cloned it and added ' | ||
| 'with `azdev extension repo add`?'.format(repo_name)) | ||
| raise CLIError( | ||
| f'Unable to find `{repo_name}` repo. Have you cloned it and added with `azdev extension repo add`?' | ||
| ) |
Author
There was a problem hiding this comment.
Function create_extension refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
Comment on lines
-77
to
+87
| heading('Creation of {} successful!'.format(package_name)) | ||
| heading(f'Creation of {package_name} successful!') | ||
| display('Getting started:') | ||
| display('\n To see your new commands:') | ||
| display(' `az {} -h`'.format(group_name)) | ||
| display(f' `az {group_name} -h`') | ||
| display('\n To discover and run your tests:') | ||
| display(' `azdev test {} --discover`'.format(group_name)) | ||
| display(f' `azdev test {group_name} --discover`') | ||
| display('\n To identify code style issues (there will be some left over from code generation):') | ||
| display(' `azdev style {}`'.format(group_name)) | ||
| display(f' `azdev style {group_name}`') | ||
| display('\n To identify CLI-specific linter violations:') | ||
| display(' `azdev linter {}`'.format(group_name)) | ||
| display(f' `azdev linter {group_name}`') |
Author
There was a problem hiding this comment.
Function _display_success_message refactored with the following changes:
- Replace call to format with f-string [×5] (
use-fstring-for-formatting)
Comment on lines
-145
to
+150
| display('MODULES: {}'.format(', '.join([x[0] for x in modules]))) | ||
| display(f"MODULES: {', '.join([x[0] for x in modules])}") |
Author
There was a problem hiding this comment.
Function verify_versions refactored with the following changes:
- Replace call to format with f-string [×2] (
use-fstring-for-formatting) - Merge dictionary updates via the union operator (
dict-assign-update-to-union)
Comment on lines
-184
to
+200
| mod = '{}{}'.format(COMMAND_MODULE_PREFIX, mod) | ||
| mod = f'{COMMAND_MODULE_PREFIX}{mod}' | ||
|
|
||
| setup_path = find_files(mod_path, 'setup.py') | ||
| with open(setup_path[0], 'r') as f: | ||
| local_version = 'Unknown' | ||
| for line in f.readlines(): | ||
| if line.strip().startswith('VERSION'): | ||
| local_version = version_pattern.match(line).group('ver') | ||
| break | ||
| local_version = next( | ||
| ( | ||
| version_pattern.match(line)['ver'] | ||
| for line in f | ||
| if line.strip().startswith('VERSION') | ||
| ), | ||
| 'Unknown', | ||
| ) |
Author
There was a problem hiding this comment.
Function _get_module_versions refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting) - Use the built-in function
nextinstead of a for-loop (use-next) - Iterate over files directly rather than using readlines() (
use-file-iterator) - Replace m.group(x) with m[x] for re.Match objects (
use-getitem-for-re-match-groups)
Comment on lines
-213
to
+221
| result = pip_cmd('download {} --no-deps -d {}'.format(mod, root_dir)).result | ||
| result = pip_cmd(f'download {mod} --no-deps -d {root_dir}').result |
Author
There was a problem hiding this comment.
Function _compare_module_against_pypi refactored with the following changes:
- Replace call to format with f-string [×5] (
use-fstring-for-formatting) - Replace m.group(x) with m[x] for re.Match objects [×2] (
use-getitem-for-re-match-groups) - Swap if/else branches of if expression to remove negation (
swap-if-expression) - Merge nested if conditions (
merge-nested-ifs)
Comment on lines
-284
to
+301
| with open(file1, 'r') as f1, open(file2, 'r') as f2: | ||
| errors.append(os.linesep.join(diff for diff in difflib.context_diff(f1.readlines(), f2.readlines()))) | ||
| with (open(file1, 'r') as f1, open(file2, 'r') as f2): | ||
| errors.append( | ||
| os.linesep.join( | ||
| iter(difflib.context_diff(f1.readlines(), f2.readlines())) | ||
| ) | ||
| ) |
Author
There was a problem hiding this comment.
Function _diff_files refactored with the following changes:
- Simplify generator expression (
simplify-generator)
Comment on lines
-304
to
+322
| elif len(dirs_cmp.right_only) == 1 and dirs_cmp.right_only[0].endswith('.whl'): | ||
| pass | ||
| else: | ||
| elif len(dirs_cmp.right_only) != 1 or not dirs_cmp.right_only[ | ||
| 0 | ||
| ].endswith('.whl'): |
Author
There was a problem hiding this comment.
Function _compare_folders refactored with the following changes:
- Remove empty elif clause (
remove-pass-elif) - Replace assignment with augmented assignment (
aug-assign)
Comment on lines
-329
to
+345
| for line in f.readlines(): | ||
| for line in f: |
Author
There was a problem hiding this comment.
Function _extract_dependencies refactored with the following changes:
- Iterate over files directly rather than using readlines() (
use-file-iterator) - Replace call to format with f-string (
use-fstring-for-formatting)
Comment on lines
-351
to
+367
| mismatch[key] = '{} != {}'.format(val, deps2[key]) | ||
| mismatch[key] = f'{val} != {deps2[key]}' |
Author
There was a problem hiding this comment.
Function _compare_dependencies refactored with the following changes:
- Replace call to format with f-string [×5] (
use-fstring-for-formatting)
Comment on lines
-17
to
+26
| 'branch': 'restapi_auto_{}/resource-manager'.format(module) | ||
| 'branch': f'restapi_auto_{module}/resource-manager', | ||
| } | ||
| pip_cmd('install "git+https://github.com/Azure/azure-sdk-for-python{pr}@{branch}' | ||
| '#egg=azure-mgmt-{module}&subdirectory=azure-mgmt-{module}"'.format(**kwargs), | ||
| show_stderr=True, | ||
| message='Installing draft SDK for azure-mgmt-{}...'.format(module)) | ||
| pip_cmd( | ||
| 'install "git+https://github.com/Azure/azure-sdk-for-python{pr}@{branch}' | ||
| '#egg=azure-mgmt-{module}&subdirectory=azure-mgmt-{module}"'.format( | ||
| **kwargs | ||
| ), | ||
| show_stderr=True, | ||
| message=f'Installing draft SDK for azure-mgmt-{module}...', | ||
| ) |
Author
There was a problem hiding this comment.
Function install_draft_sdk refactored with the following changes:
- Replace call to format with f-string [×2] (
use-fstring-for-formatting)
Comment on lines
-28
to
+31
| raise CLIError('{} is not a valid path.'.format(path)) | ||
| raise CLIError(f'{path} is not a valid path.') | ||
| _check_repo(path) | ||
| if file_name not in os.listdir(path): | ||
| raise CLIError("'{}' does not contain the expected file '{}'".format(path, file_name)) | ||
| raise CLIError(f"'{path}' does not contain the expected file '{file_name}'") |
Author
There was a problem hiding this comment.
Function _check_path refactored with the following changes:
- Replace call to format with f-string [×2] (
use-fstring-for-formatting)
| def _check_repo(path): | ||
| if not os.path.isdir(os.path.join(path, '.git')): | ||
| raise CLIError("'{}' is not a valid git repository.".format(path)) | ||
| raise CLIError(f"'{path}' is not a valid git repository.") |
Author
There was a problem hiding this comment.
Function _check_repo refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
Comment on lines
-305
to
+314
| if not yes: | ||
| if exists: | ||
| if not prompt_y_n( | ||
| "{} already exists. You may need to bump the extension version. Replace?".format(whl_file), | ||
| default='n'): | ||
| logger.warning("Skipping '%s'...", whl_file) | ||
| continue | ||
| if ( | ||
| not yes | ||
| and exists | ||
| and not prompt_y_n( | ||
| f"{whl_file} already exists. You may need to bump the extension version. Replace?", | ||
| default='n', | ||
| ) | ||
| ): | ||
| logger.warning("Skipping '%s'...", whl_file) | ||
| continue |
Author
There was a problem hiding this comment.
Function publish_extensions refactored with the following changes:
- Merge nested if conditions [×2] (
merge-nested-ifs) - Replace call to format with f-string (
use-fstring-for-formatting)
Comment on lines
-29
to
+31
| raise AssertionError("Expected 1 module to load starting with " | ||
| "'{}': got {}".format(EXTENSION_PREFIX, pos_mods)) | ||
| raise AssertionError( | ||
| f"Expected 1 module to load starting with '{EXTENSION_PREFIX}': got {pos_mods}" | ||
| ) |
Author
There was a problem hiding this comment.
Function _get_extension_modname refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
Comment on lines
-53
to
+55
| azext_metadata = _get_azext_metadata(ext_dir) | ||
| if azext_metadata: | ||
| metadata.update(azext_metadata) | ||
| if azext_metadata := _get_azext_metadata(ext_dir): | ||
| metadata |= azext_metadata |
Author
There was a problem hiding this comment.
Function get_ext_metadata refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression) - Merge dictionary updates via the union operator (
dict-assign-update-to-union)
Comment on lines
-74
to
+76
| assert r.status_code == 200, "Request to {} failed with {}".format(url, r.status_code) | ||
| assert r.status_code == 200, f"Request to {url} failed with {r.status_code}" | ||
| except AssertionError: | ||
| raise CLIError("unable to download (status code {}): {}".format(r.status_code, url)) | ||
| raise CLIError(f"unable to download (status code {r.status_code}): {url}") |
Author
There was a problem hiding this comment.
Function get_whl_from_url refactored with the following changes:
- Replace call to format with f-string [×2] (
use-fstring-for-formatting)
Comment on lines
-46
to
+65
| if help_files_not_found or help_files_to_add_to_map: | ||
| error_lines = [] | ||
| error_lines.append('Errors whilst verifying {}!'.format(DOC_MAP_NAME)) | ||
| if help_files_not_found: | ||
| error_lines.append('The following files are in {} but do not exist:'.format(DOC_MAP_NAME)) | ||
| error_lines += help_files_not_found | ||
| if help_files_not_found: | ||
| error_lines = [ | ||
| f'Errors whilst verifying {DOC_MAP_NAME}!', | ||
| f'The following files are in {DOC_MAP_NAME} but do not exist:', | ||
| ] | ||
| error_lines += help_files_not_found | ||
| if help_files_to_add_to_map: | ||
| error_lines.append('The following files should be added to {}:'.format(DOC_MAP_NAME)) | ||
| error_lines.append(f'The following files should be added to {DOC_MAP_NAME}:') | ||
| error_lines += help_files_to_add_to_map | ||
| error_msg = '\n'.join(error_lines) | ||
| raise CLIError(error_msg) | ||
| display('Verified {} OK.'.format(DOC_MAP_NAME)) | ||
| elif help_files_to_add_to_map: | ||
| error_lines = [ | ||
| f'Errors whilst verifying {DOC_MAP_NAME}!', | ||
| f'The following files should be added to {DOC_MAP_NAME}:', | ||
| ] | ||
| error_lines += help_files_to_add_to_map | ||
| error_msg = '\n'.join(error_lines) | ||
| raise CLIError(error_msg) | ||
| display(f'Verified {DOC_MAP_NAME} OK.') |
Author
There was a problem hiding this comment.
Function check_document_map refactored with the following changes:
- Split conditional into multiple branches (
split-or-ifs) - Merge append into list declaration [×4] (
merge-list-append) - Replace call to format with f-string [×6] (
use-fstring-for-formatting) - Remove redundant conditional [×3] (
remove-redundant-if)
Comment on lines
-67
to
+79
| missing_files = [] | ||
| for path in help_files_in_map: | ||
| if not os.path.isfile(os.path.normpath(os.path.join(cli_repo, path))): | ||
| missing_files.append(path) | ||
| return missing_files | ||
| return [ | ||
| path | ||
| for path in help_files_in_map | ||
| if not os.path.isfile(os.path.normpath(os.path.join(cli_repo, path))) | ||
| ] |
Author
There was a problem hiding this comment.
Function _map_help_files_not_found refactored with the following changes:
- Convert for loop into list comprehension (
list-comprehension) - Inline variable that is immediately returned (
inline-immediately-returned-variable)
Comment on lines
-93
to
+101
| display("Docs will be placed in {}.".format(output_dir)) | ||
| display(f"Docs will be placed in {output_dir}.") |
Author
There was a problem hiding this comment.
Function generate_cli_ref_docs refactored with the following changes:
- Replace call to format with f-string [×2] (
use-fstring-for-formatting)
Comment on lines
-111
to
+124
| display("Docs will be placed in {}.".format(output_dir)) | ||
| display(f"Docs will be placed in {output_dir}.") | ||
|
|
||
| display("Generating Docs for public extensions. Installed extensions will not be affected...") | ||
| _generate_ref_docs_for_public_exts(output_type, output_dir) | ||
|
|
||
| display("\nThe {} files are in {}".format(output_type, output_dir)) | ||
| display(f"\nThe {output_type} files are in {output_dir}") |
Author
There was a problem hiding this comment.
Function generate_extension_ref_docs refactored with the following changes:
- Replace call to format with f-string [×2] (
use-fstring-for-formatting)
Comment on lines
-131
to
+141
| raise CLIError("Cannot create output directory {} in non-existent path {}." | ||
| .format(base_dir, existing_path)) | ||
| raise CLIError( | ||
| f"Cannot create output directory {base_dir} in non-existent path {existing_path}." | ||
| ) |
Author
There was a problem hiding this comment.
Function _process_ref_doc_output_dir refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
Comment on lines
-153
to
+164
| display("\nFinished generating files for profile {} in dir {}\n".format(output_type, profile_output_dir)) | ||
| display( | ||
| f"\nFinished generating files for profile {output_type} in dir {profile_output_dir}\n" | ||
| ) |
Author
There was a problem hiding this comment.
Function _generate_ref_docs_for_all_profiles refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| self._parameters[command_name] = set() | ||
| for name in command.arguments: | ||
| self._parameters[command_name].add(name) | ||
| self._parameters[command_name] = set(command.arguments) |
Author
There was a problem hiding this comment.
Function Linter.__init__ refactored with the following changes:
- Convert for loop into set comprehension (
set-comprehension) - Replace identity comprehension with call to collection constructor (
identity-comprehension)
Comment on lines
-65
to
+63
| prefix_name = "{} {}".format(prefix_name, word).strip() | ||
| prefix_name = f"{prefix_name} {word}".strip() |
Author
There was a problem hiding this comment.
Function Linter.command_groups refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting)
| if deprecate_info: | ||
| if deprecate_info := self._command_loader.command_table[ | ||
| command_name | ||
| ].deprecate_info: |
Author
There was a problem hiding this comment.
Function Linter.command_expired refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression)
Comment on lines
-149
to
+148
| deprecate_info = group_kwargs.get('deprecate_info', None) | ||
| if deprecate_info: | ||
| if deprecate_info := group_kwargs.get('deprecate_info', None): |
Author
There was a problem hiding this comment.
Function Linter.command_group_expired refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression)
Comment on lines
-162
to
+160
| deprecate_info = parameter.get('deprecate_info', None) | ||
| if deprecate_info: | ||
| if deprecate_info := parameter.get('deprecate_info', None): |
Author
There was a problem hiding this comment.
Function Linter.parameter_expired refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression)
Author
Sourcery Code Quality Report❌ Merging this PR will decrease code quality in the affected files by 0.12%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
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.
Branch
devrefactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
devbranch, then run:Help us improve this pull request!