From 06f8f538018a0e88c36162be82a7b14f4b50ee31 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Mon, 16 Oct 2023 08:53:19 +0200 Subject: [PATCH] More fstring cleanup (#1802) More fstring cleanup SUMMARY Now that we're using tox for more of our linting tests, add flynt (encourage the use of fstrings) ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/modules/ec2_metadata_facts.py plugins/modules/s3_object.py pyproject.toml tox.ini ADDITIONAL INFORMATION flynt was mass-applied as part of the 6.0.0 but we didn't update the linters. Note: explicitly excludes ec2_metadata_facts because this is expected to be run on managed nodes as well as the controllers. This is also why ec2_metadata_facts doesn't include botocore or our usual module wrapper. Reviewed-by: Alina Buzachis --- changelogs/fragments/1802-flynt.yml | 3 +++ docs/docsite/rst/dev_guidelines.rst | 23 +++++++++++++++++++++ plugins/modules/s3_object.py | 4 ++-- pyproject.toml | 16 ++++++++++----- tox.ini | 31 ++++++++++++++++++++++------- 5 files changed, 63 insertions(+), 14 deletions(-) create mode 100644 changelogs/fragments/1802-flynt.yml diff --git a/changelogs/fragments/1802-flynt.yml b/changelogs/fragments/1802-flynt.yml new file mode 100644 index 00000000000..e9b23fd42b3 --- /dev/null +++ b/changelogs/fragments/1802-flynt.yml @@ -0,0 +1,3 @@ +minor_changes: +- ec2_metadata_facts - use fstrings where appropriate (https://github.com/ansible-collections/amazon.aws/pull/1802). +- s3_object - use fstrings where appropriate (https://github.com/ansible-collections/amazon.aws/pull/1802). diff --git a/docs/docsite/rst/dev_guidelines.rst b/docs/docsite/rst/dev_guidelines.rst index 0b1824ee95a..7519a6c9ff9 100644 --- a/docs/docsite/rst/dev_guidelines.rst +++ b/docs/docsite/rst/dev_guidelines.rst @@ -1123,3 +1123,26 @@ We also provide a ``tox`` configuration which allow you to run one specific test .. code-block:: shell $ tox -e py3 -- tests/unit/plugins/modules/test_s3_object.py + + +Code formatting +=============== + +To improve the consistency of our code we use a number of formatters and linters. These tools can +be run locally by using tox: + +.. code-block:: shell + + $ tox -m format + +.. code-block:: shell + + $ tox -m lint + +More information about each of the tools we use can be found on their websites: + +- `black `_ - opinionated code formatter. +- `isort `_ - groups and sorts imports. +- `flynt `_ - encourages the use of f-strings over alternatives such as concatination, ``%``, ``str.format()``, and ``string.Template``. +- `flake8 `_ - encourages following the PEP8 recommendations. +- `pylint `_ - a static code anaylsys tool. diff --git a/plugins/modules/s3_object.py b/plugins/modules/s3_object.py index 8f36df398f0..8fc9c8564cd 100644 --- a/plugins/modules/s3_object.py +++ b/plugins/modules/s3_object.py @@ -1381,9 +1381,9 @@ def s3_object_do_copy(module, connection, connection_v4, s3_vars): changed |= updated number_keys_updated += 1 if updated else 0 - msg = "object(s) from buckets '{0}' and '{1}' are the same.".format(src_bucket, s3_vars["bucket"]) + msg = f"object(s) from buckets '{src_bucket}' and '{s3_vars['bucket']}' are the same." if number_keys_updated: - msg = "{0} copied into bucket '{1}'".format(number_keys_updated, s3_vars["bucket"]) + msg = f"{number_keys_updated} copied into bucket '{s3_vars['bucket']}'" module.exit_json(changed=changed, msg=msg) else: # copy single object from source bucket into destination bucket diff --git a/pyproject.toml b/pyproject.toml index b78e8bd0e3c..5f6f4d55e65 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,8 +28,14 @@ src_paths = [ "tests/integration", ] -sections=["FUTURE", "STDLIB", "THIRDPARTY", "FIRSTPARTY", "ANSIBLE_CORE", "ANSIBLE_AMAZON_AWS", "ANSIBLE_COMMUNITY_AWS", "LOCALFOLDER"] -known_third_party=["botocore", "boto3"] -known_ansible_core=["ansible"] -known_ansible_amazon_aws=["ansible_collections.amazon.aws"] -known_ansible_community_aws=["ansible_collections.community.aws"] +sections = ["FUTURE", "STDLIB", "THIRDPARTY", "FIRSTPARTY", "ANSIBLE_CORE", "ANSIBLE_AMAZON_AWS", "ANSIBLE_COMMUNITY_AWS", "LOCALFOLDER"] +known_third_party = ["botocore", "boto3"] +known_ansible_core = ["ansible"] +known_ansible_amazon_aws = ["ansible_collections.amazon.aws"] +known_ansible_community_aws = ["ansible_collections.community.aws"] + +[tool.flynt] +transform-joins = true +exclude = [ + "ec2_metadata_facts", +] diff --git a/tox.ini b/tox.ini index 4d68c5069ee..e425f3a6494 100644 --- a/tox.ini +++ b/tox.ini @@ -3,10 +3,13 @@ skipsdist = True envlist = clean,ansible{2.12,2.13}-py{38,39,310}-{with_constraints,without_constraints},linters # Tox4 supports labels which allow us to group the environments rather than dumping all commands into a single environment labels = - format = black, isort - lint = complexity-report, black-lint, isort-lint, flake8-lint + format = flynt, black, isort + lint = complexity-report, black-lint, isort-lint, flake8-lint, flynt-lint units = ansible{2.12,2.13}-py{38,39,310}-{with_constraints,without_constraints} +[common] +format_dirs = {toxinidir}/plugins {toxinidir}/tests + [testenv] description = Run the test-suite and generate a HTML coverage report deps = @@ -34,34 +37,48 @@ deps = commands = -flake8 --select C90 --max-complexity 10 --format=html --htmldir={posargs:complexity} plugins [testenv:black] +depends = + flynt, isort deps = black >=23.0, <24.0 commands = - black {toxinidir}/plugins {toxinidir}/tests + black {[common]format_dirs} [testenv:black-lint] deps = {[testenv:black]deps} commands = - black -v --check --diff {toxinidir}/plugins {toxinidir}/tests + black -v --check --diff {[common]format_dirs} [testenv:isort] deps = isort commands = - isort {toxinidir}/plugins {toxinidir}/tests + isort {[common]format_dirs} [testenv:isort-lint] deps = {[testenv:isort]deps} commands = - isort --check-only --diff {toxinidir}/plugins {toxinidir}/tests + isort --check-only --diff {[common]format_dirs} [testenv:flake8-lint] deps = flake8 commands = - flake8 {posargs} {toxinidir}/plugins {toxinidir}/tests + flake8 {posargs} {[common]format_dirs} + +[testenv:flynt] +deps = + flynt +commands = + flynt {[common]format_dirs} + +[testenv:flynt-lint] +deps = + flynt +commands = + flynt --dry-run {[common]format_dirs} [testenv:linters] deps =