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 precommit hook #129

Merged
merged 8 commits into from
Oct 24, 2022
Merged

Add precommit hook #129

merged 8 commits into from
Oct 24, 2022

Conversation

programmiri
Copy link
Contributor

About this change - What it does

Introduces husky and lint-staged to run scripts on pre-commit if there are changes in coral.

Resolves: #112

Why this way

  • prepare script in package.json is needed in this form ('cd .. && husky install coral/.husky') because .git folder is not on the same level as the package.json (see documentation)
  • adds lint-staged page to enable us to control which scripts have to run dependent on where a file changes:
    • right now we only need to run the linting pre-commit hooks on changes in TypeScript and TSX files, since we have no linting for markdown etc.
  • in pre-commit is a check to see if the staged files are in the coral directory, so we don't run tests when something outside coral changes.

@programmiri programmiri changed the base branch from main to coral October 20, 2022 15:42
Copy link
Contributor

@SmuliS SmuliS left a comment

Choose a reason for hiding this comment

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

Couple of things:

  1. move lint-staged depedency under coral.
  2. Couple of trivial comments in the actual pre-commit file

Lets also keep our minds open for https://pre-commit.com/, which clould also establish a base for git hooks for Java changes.

package.json Outdated Show resolved Hide resolved
coral/.husky/pre-commit Outdated Show resolved Hide resolved
coral/.husky/pre-commit Outdated Show resolved Hide resolved
@SmuliS
Copy link
Contributor

SmuliS commented Oct 21, 2022

Is a general personal note: I would have not introduced lint-staged as a depedency as I think similar behaviour can be achieved with bash tooling.

coral/.husky/pre-commit Outdated Show resolved Hide resolved
@programmiri
Copy link
Contributor Author

Is a general personal note: I would have not introduced lint-staged as a depedency as I think similar behaviour can be achieved with bash tooling.

Adding here what we decided about this in a call to be transparent:

  • we will leave lint-staged for now, because it improves readability to enable others to get an overview about the processes more quick when compared to bash scripts
  • we'll come back to the topic pre-commit-hooks later and check out pre-commit, which looks like a promising tool to only have one place where we define things while being very readable 💃

@programmiri programmiri force-pushed the add-precommit-hook branch 2 times, most recently from 3b55942 to f9c6979 Compare October 21, 2022 11:11
@programmiri programmiri self-assigned this Oct 21, 2022
- remove lint-staged that was accidentally installed on root instead of coral.
- add lint-staged package to coral
- add script to package.json
- update pre-commit script to always change to git-root/coral
- use variable for frontend-root to make it easier to change that later
- remove echo
- use --prefix instead of changing directory
lint-staged passes changed filenames as argument. If files
are passed as argument `tsc` does not use tsconfig.json,
but CLI inline config.

Also run plain prettier and eslint commands with lint-staged.
the `pnpm lint` does not use files as argument.

Refers: #112
@SmuliS SmuliS merged commit b5da1c8 into coral Oct 24, 2022
@SmuliS SmuliS deleted the add-precommit-hook branch October 24, 2022 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged Acknowledged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🪸 ui-rewrite: Add pre-commit hooks linting, formatting, testing
2 participants