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

Make the pre-commit hook fix formatting #147

Merged
merged 97 commits into from
Aug 1, 2023
Merged

Make the pre-commit hook fix formatting #147

merged 97 commits into from
Aug 1, 2023

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented Jul 27, 2023

Changes

  • The pre-commit hook is now like Gutenberg's. It fixes formatting when possible and stages the fix before the commit. If it can't fix it, it blocks the commit.
  • Removes the husky pre-commit hook package. It was outdated, so we either had to migrate or remove it.
  • So to avoid husky, this adds the pre-commit hook with a git command in package.json prepare
  • I'll still have to iterate on this. But maybe we could use this in other repos if it helps.

Testing instructions

  1. Do a fresh clone of this repo
  2. git checkout remove/husky
  3. composer i && nvm use && npm i
  4. Create a JS formatting error. Maybe remove a space inside parentheses in a JS file.
  5. Stage that change
  6. git commit -m "Should be fixed"
  7. Expected: That formatting error is fixed before being committed.

@@ -112,7 +111,7 @@
"lint:php": "composer lint",
"lint:php:fix": "composer lint-fix",
"packages-update": "wp-scripts packages-update",
"precommit": "./bin/pre-commit.sh",
"prepare": "git config --local core.hooksPath .githooks",
Copy link
Contributor Author

@kienstra kienstra Jul 27, 2023

Choose a reason for hiding this comment

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

This won't always work, but I like it more than importing husky and updating its versions.

Also, we won't need a .husky/ directory, only .githooks/.

I don't like directories named after vendors, but we still have some.

@kienstra kienstra changed the title Replace husky with a vanilla git commit Replace husky with a vanilla git command Jul 27, 2023
@kienstra
Copy link
Contributor Author

There aren't this many commits for this work, those are because this PR targeted another branch, and I merged that branch in.

@kienstra kienstra marked this pull request as ready for review July 27, 2023 20:03
@kienstra kienstra requested review from dreamwhisper and mike-day and removed request for dreamwhisper and mike-day July 27, 2023 20:06
@kienstra
Copy link
Contributor Author

Sorry, this actually isn't ready for review. It has some bugs.

@kienstra kienstra marked this pull request as draft July 27, 2023 20:19
<directory suffix=".php">./tests/php/Integration/Helpers/</directory>
<directory prefix="Test" suffix=".php">./tests/php/Integration/</directory>
</testsuite>
</testsuites>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not related, I just wanted to indent this with tabs, like phpcs.xml

@kienstra kienstra changed the title Replace husky with a vanilla git command Make the pre-commit hook fix formatting Jul 28, 2023
@kienstra
Copy link
Contributor Author

This PR din't have that many commits

Those are because this was branched off another PR, and I merged that in.

@kienstra kienstra marked this pull request as ready for review July 28, 2023 23:29
executor:
name: php
docker:
- image: cimg/php:8.2-node
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make linting pass

@kienstra
Copy link
Contributor Author

Sorry, this is ready for review again.

@kienstra
Copy link
Contributor Author

Sorry for yet another GCB PR!

Copy link

@mike-day mike-day left a comment

Choose a reason for hiding this comment

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

This is working well, @kienstra! It's a nice touch to do this without relying on husky.


Before making the commit, I had linting errors:

Screenshot 2023-08-01 at 10 29 14 AM

After making the commit, the auto-fixable errors were all fixed and the commit succeeded:

Screenshot 2023-08-01 at 10 29 36 AM

@kienstra
Copy link
Contributor Author

kienstra commented Aug 1, 2023

Hi @mike-day,
Wow, great to see how it fixed those linting errors on your machine!

Thanks for the screenshots and for testing this.

@kienstra kienstra merged commit 6f0e124 into develop Aug 1, 2023
@kienstra kienstra deleted the remove/husky branch August 1, 2023 16:14
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.

2 participants