Skip to content

Add pre commit hook for linting JS#8862

Merged
oskosk merged 7 commits intomasterfrom
add/pre-commit-hook
Feb 16, 2018
Merged

Add pre commit hook for linting JS#8862
oskosk merged 7 commits intomasterfrom
add/pre-commit-hook

Conversation

@oskosk
Copy link
Copy Markdown
Contributor

@oskosk oskosk commented Feb 16, 2018 •
edited
Loading

Fixes the surprise when looking Travis fail due to JS linting errors.

Testing instructions

  1. checkout to this branch.
  2. Run yarn so new dependencies are installed
  3. Introduce a local change in a file (e.g. _inc/client/main.jsx ) adding a lint problem. (An unused var or a let that should be const)
  4. Attempt to commit the change.
  5. Expect the commit to fail.
  6. Fix the error and add a change that does not present problems (an extra space between imports or so).
  7. Commit the change and expect to not be blocked by the linter.

Changes proposed in this Pull Request:

  • Upgrades all eslint rules to errors expect for react/prefer-es6-class and react/no-bind. Those remain as wranings.
  • Adds no-unused-vars rule`
  • Adds husky as dependency.
  • Makes git ignore .eslintcache.
  • Adds package.json script precommit to run bin/pre-commit-hook.js

Changelog entry

  • We added a pre-commit hook for linting the Jetpack Admin Page source code. This aids in finding common syntax and style problems before having to wait for automated tests to run.

@oskosk oskosk added [Status] Needs Review This PR is ready for review. Admin Page React-powered dashboard under the Jetpack menu [Type] Janitorial labels Feb 16, 2018
@oskosk oskosk requested a review from a team as a code owner February 16, 2018 18:59
@oskosk oskosk force-pushed the add/pre-commit-hook branch from 040d2f5 to 7c59d5d Compare February 16, 2018 19:05
dereksmart
dereksmart approved these changes Feb 16, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@dereksmart dereksmart left a comment

Choose a reason for hiding this comment

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

Very nice. Works as expected. Let's try it out!

@dereksmart dereksmart added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Feb 16, 2018
jeherve
jeherve approved these changes Feb 16, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

seems to work for me too 🎉

@oskosk oskosk added this to the 5.9 milestone Feb 16, 2018
@oskosk oskosk merged commit d944f9c into master Feb 16, 2018
@oskosk oskosk deleted the add/pre-commit-hook branch February 16, 2018 21:42
oskosk added a commit that referenced this pull request Feb 27, 2018
@oskosk
Add changelog entry for #8862
c6035f5
oskosk added a commit that referenced this pull request Feb 27, 2018
@oskosk
Changelog 5.9: running changelog (#8819)
1174ef5 
* update changelog.txt

* Update readme.txt with scaffolding for 5.9 changelog and release draft shortlink

* Add changelog entry for #8243

* Add changelog entry for #8296

* Add changelog entry for #8367

* Add changelog entry for #8686

* Add changelog entry for #8707

* Add changelog entry for #8709 and #8714

* Add changelog entry for #8729

* Add changelog entry for #8777

* Add changelog entry for #8780

* Add changelog entry for #8786

* Add changelog entry for #8787

* Add changelog entry for #8801 #8805 #8832 #8865 and #8804

* Add changelog entry for #8817

* Add changelog entry for #8822

* Add changelog entry for #8823

* Add changelog entry for #8829

* Add changelog entry for #8834

* move some items to major enhancements

* Add changelog entry for #8836

* Add changelog entry for #8839

* Add changelog entry for #8861

* Add changelog entry for #8862

* Add changelog entry for #8863

* Add changelog entry for #8866

* Add changelog entry for #8870

* Add changelog entry for #8874

* Add changelog entry for #8875

* Add changelog entry for #8881

* Add changelog entry for #8890

* Add changelog entry for #8911

* Add changelog entry for #8927

* Add changelog entry for #8931

* Add changelog entry for #8933

* Add changelog entry for #8930

* fix wording

* typo

* minor fixes

* replace partner scripts for Jetpack Start in changelog entry

* Update to-test.md

* Update to-test.md

* minor style fixes to to-test.md

* minor style fixes to to-test.md

* minor fixes on to-test.md

* Add changelog entry for #8868

* Add changelog entry for #8844

* Add changelog entry for #8664

* Add changelog entry for #8935

* Add changelog entry for #8425

* Add changelog entry for #8625
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

@dereksmart dereksmart dereksmart approved these changes

Assignees

No one assigned

Labels

Admin Page React-powered dashboard under the Jetpack menu Touches WP.com Files [Type] Janitorial

Projects

None yet

Milestone

5.9

Development

Successfully merging this pull request may close these issues.

5 participants

@oskosk @jeherve @dereksmart @kraftbj @matticbot