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

Add prettier #389

Merged
merged 8 commits into from
Dec 17, 2023
Merged

Add prettier #389

merged 8 commits into from
Dec 17, 2023

Conversation

js0mmer
Copy link
Member

@js0mmer js0mmer commented Dec 11, 2023

Description

  • Adds prettier and a husky precommit hook that calls lint-staged
  • Next step would be to run formatting on the repo, otherwise there will be large diffs when reviewing PRs.
    • I think this would be better to implement in a separate PR, that way changes from this PR can be added to existing branches first which can help mitigate merge conflicts

Screenshots

Example commit
Screenshot 2023-12-07 203350

Steps to verify/test this change:

  • Check out locally, npm install in root, then make a change and a test commit

Final Checks:

  • Verify successful deployment
  • Delete branch

(optional)

  • Write tests
  • Write documentation

Issues

Closes #370

@js0mmer js0mmer self-assigned this Dec 11, 2023
@js0mmer js0mmer requested a review from Awesome-E December 17, 2023 21:32
Copy link
Member

@Awesome-E Awesome-E left a comment

Choose a reason for hiding this comment

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

image
At least on MacOS, the hook doesn't run because it's not given executable permissions

lint-staged.config.cjs Outdated Show resolved Hide resolved
prettier.config.cjs Outdated Show resolved Hide resolved
@js0mmer js0mmer requested a review from Awesome-E December 17, 2023 21:53
Copy link

Deployed staging instance to https://staging-389.peterportal.org

Copy link
Member

@Awesome-E Awesome-E left a comment

Choose a reason for hiding this comment

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

Formatting looks like it's working now!

I think one more thing just to be cautious of – since all commits will autoformat the entire file now, we should really make an effort to resolve the other PRs because any change to the same files will lead to lots of merge conflicts

@js0mmer js0mmer merged commit 0448f29 into master Dec 17, 2023
2 checks passed
@js0mmer js0mmer deleted the add-prettier branch December 17, 2023 22:05
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 prettier
2 participants