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

Prettier pre commit hook #419

Draft
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

dakaza98
Copy link
Member

@dakaza98 dakaza98 commented Mar 28, 2023

This PR sets up a prettier pre commit hook. Everytime a commit is made, the pre hook will automatically format all files that have been staged in the commit so that the code always gets formatted even if someone hasn't installed prettier in their editor

I will make a separate PR that will format all existing code to the prettier configuration once this PR has been merged

Closes #386

Copy link
Collaborator

@sgronlund sgronlund left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@albinantti albinantti left a comment

Choose a reason for hiding this comment

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

I feel like we should discuss what convention we actually use before implementing something like this (and which linter we use for that matter).

Is prettier setup to use the editorconfig file as rules? I can't see where this setting is enabled, and if not why is the prettierrc file empty?

I disagree with the 2-space indentation, and feel like there are other things we should consider linting for such as single our double quotation marks for strings. Might be worth discussing this outside of a github pull request

@albinantti
Copy link
Member

Can add that I believe we elected (then immediately forgot) to use Airbnb's style convention when we did our bachelor thesis work

@dakaza98
Copy link
Member Author

Prettier automatically uses some of the rules in editorconfig if it detects an editorconfig file. The editorconfig file also sets up the editor to use the correct settings from the start.

The reason the .prettierrc.json file is empty is because prettier recommends that the file exists even though it has no configuration since it can let other tools know that we are using prettier.

The nice thing about prettier is that it's default formatting rules makes the code readable and avoids long lines without having to decide on each rule it has.

Having 2 spaces as indentation is very common in React apps since most JSX functions tend to become very nested which makes them difficult to read, having only 2 spaces helps out with that issue

@dakaza98 dakaza98 marked this pull request as draft March 29, 2023 08:57
@dakaza98
Copy link
Member Author

We will implement Airbnb's style with Eslint before adding the prettier pre-commit hook

. "$(dirname -- "$0")/_/husky.sh"

cd frontend
npx lint-staged
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
npx lint-staged
yarn lint-staged

We should use yarn instead of npx.

"prepare": "cd .. && husky install frontend/.husky"
},
"lint-staged": {
"**/*": "prettier --write --ignore-unknown"
Copy link
Collaborator

Choose a reason for hiding this comment

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

When implementing ESLint, a pre-commit hook should be added for ESLint.

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.

Add pretter pre-commit hook
4 participants