Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pre-commit prettier version #694

Conversation

PeteCoward
Copy link
Contributor

@PeteCoward PeteCoward commented May 11, 2023

fixes #693

Upgrading the prettier check to it's latest version v2.7.1 removed these warnings.

The changes in 53d9d8e are the result of the prettier upgrade and consist of

  • the change to .github/workflows/test.yml mentioned in the issue
  • trailing commas in object initialisers
  • wrapping arguments to function shorthand notation in brackets
  • a space between function and ( - function (...)

Also delegates prettier check to pre-commit as per example https://github.com/wagtail/cookiecutter-wagtail-package/pull/47/files

@@ -32,7 +32,7 @@ repos:
- flake8-bugbear
- flake8-comprehensions
- repo: https://github.com/pre-commit/mirrors-prettier
rev: 'v1.18.2'
rev: 'v2.7.1'
Copy link
Collaborator

Choose a reason for hiding this comment

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

@PeteCoward any chance you could drop the linting dependencies from package.json while at this? This way we keep it just in pre-commit.

https://github.com/wagtail/cookiecutter-wagtail-package/pull/47/files has an example.

If not, will make that a follow-up task

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had a go.

I snuck in a change from pip install -e .[testing] -U to pip install -e '.[testing]' -U too, which caught me out following the README.

I did not make the change to the pre-commit run all files README comment, as I was not sure of the rationale behind it.

@zerolab
Copy link
Collaborator

zerolab commented Jun 2, 2023

Thank you for this, @PeteCoward
Merged manually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pre-commit checks have warnings and change .github/workflows/test.yml
2 participants