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

chore: add TypeScript styles and norms #24

Merged
merged 12 commits into from
May 23, 2023

Conversation

cguedes
Copy link
Collaborator

@cguedes cguedes commented May 22, 2023

Add recommended rules from #18

  • 120 char width (prettier)
  • Add eslint deps and rules (.eslintrc)
  • Fix files using: simple-import-sort
  • Manual fixes for any

I've also added the ./vscode/settings.json and ./vscode/extensions.json with recommended settings. It makes easier to comply with rules after a git clone.

@cguedes cguedes marked this pull request as ready for review May 22, 2023 10:44
@cguedes cguedes requested review from sehyod and danvk May 22, 2023 10:45
@cguedes cguedes assigned cguedes and unassigned cguedes May 22, 2023
@cguedes
Copy link
Collaborator Author

cguedes commented May 22, 2023

@sergioramos moved eslint rules to package.json

@cguedes cguedes requested a review from sergioramos May 22, 2023 11:00
@hammer
Copy link
Contributor

hammer commented May 22, 2023

Should we use pre-commit to automate the enforcement of these guidelines?

@cguedes
Copy link
Collaborator Author

cguedes commented May 22, 2023

@hammer this project was already using (from Tauri) husky.
I've fixed the commit-hook and now (after yarn install) should be working.

I've configured the project to use, file based commit hook (.husky/pre-commit), to run the already configured lint-staged.

@danvk
Copy link
Collaborator

danvk commented May 22, 2023

My two cents: I've always preferred pre-push hooks (rather than pre-commit) so that making local commits is instant but your code is always nicely formatted and linted by the time anyone else would have to look at it.

@cguedes
Copy link
Collaborator Author

cguedes commented May 22, 2023

@danvk I like that also. You are right, the local commits would be slower.

I will update to use a pre-push that fails on lint errors (and maybe in the future with tests).

@cguedes
Copy link
Collaborator Author

cguedes commented May 23, 2023

Added pre-push that runs lint:check

Copy link
Collaborator

@sehyod sehyod left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you for this! I made a comment about lint-staged, that I think we can remove, but nothing blocking

"eslint-plugin-react": "^7.32.2",
"eslint-plugin-react-hooks": "^4.6.0",
"eslint-plugin-simple-import-sort": "^10.0.0",
"husky": "^8.0.0",
"lint-staged": "^13.2.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need lint-staged if we only setup pre-push hooks?

@sergioramos sergioramos changed the title Add TypeScript styles and norms chore: add TypeScript styles and norms May 23, 2023
@sergioramos sergioramos merged commit 8f4772f into main May 23, 2023
@sergioramos sergioramos deleted the feature-typescript-styles-norms branch May 23, 2023 09:58
@cguedes cguedes mentioned this pull request May 23, 2023
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.

None yet

5 participants