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 and run prettier command via lint-staged #448

Merged
merged 7 commits into from
Oct 16, 2023

Conversation

maskeynihal
Copy link
Contributor

@maskeynihal maskeynihal commented Oct 10, 2023

Type of Change

  • Build Scripts: Add prettier with husky for code formatting
  • Tool Source: JS and Vue
  • Something else: Added prettier which runs with lint-staged.

What issue does this relate to?

resolves #447

What should this PR do?

  • Add new package prettier
  • Run the prettier using lint-staged when creating a new git commit
  • npm run prettier should style the issue by prettier
  • npm run prettier:check should check if there is any error and give a list of files that have errors.

What are the acceptance criteria?

  • prettier should run in only the files that are changed when a new git commit is added.
  • test prettier CI should run on PR and it should pass before merging the code.

@maskeynihal
Copy link
Contributor Author

here are the changes that are made by prettier. maskeynihal#1

should I also push the changes? @MattIPv4?

the test failing should fixed by the changes.

@MattIPv4
Copy link
Member

I think you'll probably want to add a dedicated test step for prettier -- eslint is definitely not flagging all the issues.

And yeah, a commit with all the changes prettier wants would be good as well.

run prettier and eslint fix on whole project with .js and .vue extension
@maskeynihal
Copy link
Contributor Author

@MattIPv4 All the checks are passing.

I think you'll probably want to add a dedicated test step for prettier

I didn't get what you meant by this.

@MattIPv4
Copy link
Member

You'll want to add a new GitHub Actions test check for prettier -- eslint did fail, but it definitely did not fail with everything that prettier wanted to change.

@maskeynihal maskeynihal force-pushed the add-prettier branch 2 times, most recently from 6a474d4 to 1aa8b23 Compare October 12, 2023 06:11
@maskeynihal
Copy link
Contributor Author

maskeynihal commented Oct 12, 2023

@MattIPv4 I have added github action to check code by prettier.

Should we restrict the use of the "prettier" check to only .js and .vue files, or should we also apply it to other file types such as .md, .json, and so on?

Thanks for taking the time to look into the PR.

@MattIPv4 MattIPv4 self-requested a review October 12, 2023 22:35
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Member

@MattIPv4 MattIPv4 left a comment

Choose a reason for hiding this comment

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

LGTM, tyvm!

@MattIPv4 MattIPv4 merged commit dbc954e into digitalocean:master Oct 16, 2023
4 checks passed
@maskeynihal
Copy link
Contributor Author

@MattIPv4 Would you mind adding the "hacktoberfest" tag to this PR if you can? It would be fantastic for keeping track of contributions during Hacktoberfest, but don't worry if you can't. I'm thrilled to contribute either way!

@MattIPv4
Copy link
Member

MattIPv4 commented Oct 16, 2023

The repository already has the hacktoberfest topic, that is all that is needed for contributions to be counted for it. Adding a hacktoberfest label to a PR does absolutely nothing in the context of Hacktoberfest.

@maskeynihal
Copy link
Contributor Author

@MattIPv4 Gotcha. Thanks. Previously it was showing Not-Accepted in my profile but now it's working fine.

Thanks once again.

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