Skip to content

[CI] Add Husky + lint-staged to enforce existing lint checks at commit time#5867

Closed
thevanshit wants to merge 3 commits intosugarlabs:masterfrom
thevanshit:feat/add-husky-lint-staged
Closed

[CI] Add Husky + lint-staged to enforce existing lint checks at commit time#5867
thevanshit wants to merge 3 commits intosugarlabs:masterfrom
thevanshit:feat/add-husky-lint-staged

Conversation

@thevanshit
Copy link
Contributor

Summary

This PR adds Husky and lint-staged to automatically run the existing ESLint and Prettier checks on staged files before each commit.
The goal is to enforce the current linting and formatting workflow at commit time without introducing new rules or modifying the existing developer workflow.


Changes

  • Added husky and lint-staged as dev dependencies
  • Added "prepare": "husky install" script to enable hooks automatically after npm install
  • Configured lint-staged inside package.json to:
    • Run eslint --fix on *.{js,mjs} files
    • Run prettier --write on staged files
  • Added .husky/pre-commit hook to trigger lint-staged

Behavior

  • git commit now runs lint and formatting checks on staged files only
  • Fixable issues (spacing, quotes, semicolons) are auto-corrected before commit
  • Commits are blocked if lint errors remain
  • The hook can be bypassed with git commit --no-verify
  • Existing npm run lint workflow remains unchanged
  • No impact on CI

Testing

  • Verified lint-staged runs only on staged files
  • Verified auto-fix behavior for formatting issues
  • Verified commit fails when lint errors remain
  • All existing tests pass

Closes #5864

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@Anshikakalpana
Copy link

Hi @thevanshit , I'm the one who opened this issue and had planned to implement it. I noticed the PR also includes unrelated commits (chore: remove debug console statements, Adding a PERFORMANCE.md file) — it's generally good practice to keep PRs focused on a single change. Happy to collaborate or help review if needed!

@thevanshit thevanshit force-pushed the feat/add-husky-lint-staged branch from a093104 to 007ab63 Compare February 23, 2026 14:09
@github-actions
Copy link
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

palette.test.js
logo.test.js
logoconstants.test.js

@thevanshit
Copy link
Contributor Author

Hi, @Anshikakalpana

I have cleaned the PR so it now contains only the Husky + lint-staged configuration changes.
I also verified that the failing Jest tests (palette.test.js, logo.test.js, logoconstants.test.js) fail on upstream/master as well, so they appear unrelated to this PR.
One thing I noticed is that adding dev dependencies caused large changes in package-lock.json due to dependency resolution.

If maintainers prefer, I can:

Happy to refine further

@thevanshit thevanshit force-pushed the feat/add-husky-lint-staged branch from 007ab63 to a5405f2 Compare February 23, 2026 17:08
@github-actions
Copy link
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

palette.test.js
logo.test.js
logoconstants.test.js

- Add MIN_HIGHLIGHT_DURATION_MS to logoconstants test expectations
- Remove incorrect console expectation in palette.test.js
- Add stage.update mock to logo.test.js
- Fix delayExecution mock to handle missing callback

Note: These failures existed on upstream/master before this PR.
Reduced logo.test.js failures from 11 to 5.
@thevanshit thevanshit force-pushed the feat/add-husky-lint-staged branch from a5405f2 to 3e25648 Compare February 23, 2026 17:45
@github-actions
Copy link
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

logo.test.js

- Fix initMediaDevices: update logo.deps.Tone directly in test
- Fix dispatchTurtleSignals tests: update logo.deps.utils.delayExecution
- Fix doStopTurtles: update logo.deps.instruments and turtleList
- Fix constructor deps test: add missing required fields

All 181 tests now pass (logo: 71, palette: 103, logoconstants: 7)
@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@thevanshit
Copy link
Contributor Author

Hi @Anshikakalpana and maintainers,

I’ve addressed the feedback and refined this PR:
Cleaned the history so it now contains only the Husky + lint-staged implementation related to #5864
Resolved the failing Jest tests (including pre-existing logo test issues)
Verified CI is now fully passing

The goal remains to enforce the existing ESLint + Prettier workflow locally without introducing new rules or changing current contributor workflows.
If everything looks good, this should now be ready for review and merge. Please let me know if any further adjustments are preferred.

Thanks!

@omsuneri
Copy link
Member

no one is going to review this PR please dont raise such a bulky pr Thanks !!

@omsuneri omsuneri closed this Feb 25, 2026
@thevanshit thevanshit deleted the feat/add-husky-lint-staged branch February 25, 2026 18:12
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.

[CI] Add Husky + lint-staged to enforce existing lint checks at commit time

3 participants