Skip to content
This repository has been archived by the owner on May 24, 2023. It is now read-only.

dev: Add pre-commit to buildservice repo #16

Merged
merged 2 commits into from
Feb 13, 2023
Merged

dev: Add pre-commit to buildservice repo #16

merged 2 commits into from
Feb 13, 2023

Conversation

blancadesal
Copy link
Member

@blancadesal blancadesal commented Jan 20, 2023

  • Changes to bash scripts are the result of making the changes required to get the linters to pass.
  • Shebangs have been changed to the portable #!/usr/bin/env style.
  • Bash 'strict mode' has been standardized across scripts.
  • Pre-commit hooks have been updated to their latest versions.

Bug: T325183
Bug: T329271

deploy.sh Outdated Show resolved Hide resolved
Changes to bash scripts are the result of making the
changes required to get the linters to pass.

Shebangs have been changed to the portable #!/usr/bin/env
style.

Bash 'strict mode' has been standardized across scripts.

Bug: T325183
Copy link
Contributor

@NdibeRaymond NdibeRaymond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why we haven't merged this? saw were @david-caro mentioned that he is having issues with pre-commit and will merge when issue is resolved

@dhinus
Copy link
Member

dhinus commented Feb 3, 2023

I tried checking out this branch and running pre-commit run --all-files and I'm also getting some failures, not sure if it's the same that @david-caro was seeing.

An unexpected error has occurred: CalledProcessError: command: ('/Users/fran/.cache/pre-commit/repoxg6ouejs/py_env-python3/bin/python', '-mpip', 'install', '.', 'toml')

[...]

      × Preparing metadata (pyproject.toml) did not run successfully.

[...]

          RuntimeError: The Poetry configuration is invalid:

@NdibeRaymond
Copy link
Contributor

I tried checking out this branch and running pre-commit run --all-files and I'm also getting some failures, not sure if it's the same that @david-caro was seeing.

An unexpected error has occurred: CalledProcessError: command: ('/Users/fran/.cache/pre-commit/repoxg6ouejs/py_env-python3/bin/python', '-mpip', 'install', '.', 'toml')

[...]

      × Preparing metadata (pyproject.toml) did not run successfully.

[...]

          RuntimeError: The Poetry configuration is invalid:

I can confirm this. Just checked on my own machine too

@david-caro
Copy link
Contributor

Doing a quick look, the error comes from the isort package that pre-commit is downloading, for which the poetry.lock file has version 1.4:

dcaro@vulcanus$ cd /home/dcaro/.cache/pre-commit/reporj2zlm63/

dcaro@vulcanus$ grep name pyproject.toml
name = "isort"
name = "material"

dcaro@vulcanus$ grep lock-version poetry.lock
lock-version = "1.1"

Looking...

@david-caro
Copy link
Contributor

Looking...

I'm using poetry version 1.3.2:

dcaro@vulcanus$ poetry --version
Poetry (version 1.3.2

@david-caro
Copy link
Contributor

Yep, there's a bug in isort 5.11.4, upgrading to 5.12.0 (latest) fixes it (reference https://github.com/PyCQA/isort/releases/tag/5.12.0, "Fix poetry pip-shims extras dependency (PyCQA/isort#2078) @jooola")

So poetry autoupdate fixes the issue, can you add a commit with it to the branch?

@david-caro
Copy link
Contributor

You can also add a link to T329271 (for the pre-commit issue)

@david-caro david-caro self-requested a review February 13, 2023 16:07
Copy link
Contributor

@david-caro david-caro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have python<3.8 for this, so latest isort works ok (when moving to gitlab we might want to double-check)

@david-caro david-caro merged commit 0bafe2f into main Feb 13, 2023
@david-caro david-caro deleted the add-pre-commit branch February 13, 2023 16:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants