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

Inactive git pre-commit Husky hook #743

Closed
1 of 2 tasks
MikeMcC399 opened this issue Jan 24, 2023 · 4 comments · Fixed by #839
Closed
1 of 2 tasks

Inactive git pre-commit Husky hook #743

MikeMcC399 opened this issue Jan 24, 2023 · 4 comments · Fixed by #839

Comments

@MikeMcC399
Copy link
Collaborator

MikeMcC399 commented Jan 24, 2023

Problem description

The git husky hook in https://github.com/cypress-io/github-action/blob/master/package.json is not being triggered:

  "husky": {
    "hooks": {
      "pre-commit": "npm run format && npm run build && git add index.js dist"
    }
  }

when changes are made to index.js they are not automatically causing dist/index.js to be updated, making the changes practically ineffective.

Husky v5.0.0 introduced breaking changes, including the fact that it no longer autoinstalls.

It seems that when Husky was updated from v4 to v7 there was no reconfiguration done in the repo.

Suggested change

To re-activate Husky would require following the migration steps (see https://typicode.github.io/husky/).

I found two disadvantages of doing this:

  1. Windows (CRLF) and Unix (LF) have incompatible line-ending formats so if commits are being done from different sources, then the format of dist/index.js would yoyo back and forwards (see Option to force Unix style output on Windows? vercel/ncc#992). The pre-commit hook would run independently of whether there had been a change to index.js and would therefore be applied to unrelated commits.
  2. GitHub Desktop (for Windows) may fail using Husky (see https://github.com/desktop/desktop/issues).

So I suggest instead to remove the Husky configuration and solve the issue through:

  1. Added documentation Incomplete build instructions in DEVELOPMENT.md #742
  2. Implementation of Add build & diff "/dist" folder workflow #706
@MikeMcC399
Copy link
Collaborator Author

If the Husky hook were to be reactivated, users on Windows could avoid issues by setting the environment variable
HUSKY=0
See Husky documentation Bypass hooks.

@MikeMcC399
Copy link
Collaborator Author

Husky 8 is more flexible in defining the hook contents, so it would be possible to commit a changed dist/ directory content only if it had changed, ignoring the end-of-line characters.

@MikeMcC399
Copy link
Collaborator Author

  • PR fix: windows build with ping.js #836 resolves the issue of build differences between Windows and unix. Once this PR is merged, the path is ready for the Husky pre-commit hook to be reactivated.

@MikeMcC399
Copy link
Collaborator Author

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 a pull request may close this issue.

1 participant