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

Enable prettier formatting #27

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ssbarnea
Copy link
Member

@ssbarnea ssbarnea commented Jan 6, 2023

  • Uses default prettier settings
  • Include vscode settings to suggest prettier extension and configure it to reformat on save (it does not force the contributor to install it but it can help if he does)
  • Update GHA to run linting after install and before build using "npm run lint".
  • Enabling pre-commit.ci github app is not required but when enabled that app can auto-fix incoming pull requests, something that our own "npm run lint" cannot do.

@fbricon
Copy link
Collaborator

fbricon commented Jan 6, 2023

can you add some instructions on how to trigger that new pre-commit hook?

@ssbarnea
Copy link
Member Author

ssbarnea commented Jan 6, 2023

@fbricon Remember to also login to https://pre-commit.ci/ and authorize pre-commit app to run on this repository. While this is not required, it is the thing that enables two important features:

  • auto-fixing of simple errors
  • once a month, it opens PRs to bump linters when new versions are available

- Uses default prettier settings
@fbricon
Copy link
Collaborator

fbricon commented Jan 11, 2023

I'm not fond of the python dependency. Can't we have 'npm lint' that calls prettier via nodejs?

I configured pre-commit app for this repo but don't understand why we need it if the CI build already calls pre-commit via npm lint

I'd be OK merging all the prettier config in this PR and discuss the pre-commit stuff in a separate one

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.

2 participants