[ci] including eslint in pre-commit #57721
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds a pre-commit hook to run eslint and prettier on the dashboard client code. While this is a good step towards automated formatting checks, the current implementation of the hook has several performance and correctness issues. I've left a comment with suggestions to improve the hook's efficiency and fix its file matching. The main concerns are that the hook runs npm ci on every commit and lints all files instead of just the changed ones, which will be very slow.
elliot-barn
changed the title
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing eslint and prettier Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating pre commit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> new comment Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> lint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> linting Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing lint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating pre-commit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing lint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating file regex Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> hold the door Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> adding eslint to precommit array Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating lint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing linter Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating pre commit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing eslint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating pre-commit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating app.tsx Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating pre commit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating pre commit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> adding a comment Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing eslint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing es lint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> adding typescript eslint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating pre-commit-config Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating empty objects Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> udpating pre -commit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating pre commit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing eslint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating pre commit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating eslint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> run the test Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing precommit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> running test Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing eslint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating pre commit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing lint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating pre-commit Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating app.tsx Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing eslint Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> testing app.tsx Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> updating app.tsx Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
elliot-barn
force-pushed
the
elliot-barn/lintupdates
branch
from
79442c9 to
4197b85
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
aslonnie
changed the title
aslonnie
approved these changes
Oct 16, 2025
Collaborator
|
cc @alanwguo |
aslonnie
reviewed
Oct 16, 2025
Collaborator
aslonnie
left a comment
aslonnie
left a comment
There was a problem hiding this comment.
seems that the new lint is failing on CI.
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
| - '@typescript-eslint/parser@5.41.0' | ||
| - '@typescript-eslint/eslint-plugin@5.41.0' | ||
| - '@typescript-eslint/parser@5.41.0' | ||
| - '@typescript-eslint/eslint-plugin@5.41.0' |
There was a problem hiding this comment.
Bug: Duplicate ESLint Dependencies in Pre-commit Hook
The eslint pre-commit hook's additional_dependencies list contains duplicate entries for @typescript-eslint/parser and @typescript-eslint/eslint-plugin, which appears to be a copy-paste error.
justinyeh1995
pushed a commit
to justinyeh1995/ray
that referenced
this pull request
Oct 20, 2025
adding eslint and prettier script to precommit before getting rid of format.sh 1 step closer to replacing scripts/format.sh with pre-commit (pre-commit is currently missing eslint) tested locally: <img width="898" height="929" alt="image" src="https://github.com/user-attachments/assets/58c77fb7-bdde-47ae-ac2b-b864334b3f30" /> --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
xinyuangui2
pushed a commit
to xinyuangui2/ray
that referenced
this pull request
Oct 22, 2025
adding eslint and prettier script to precommit before getting rid of format.sh 1 step closer to replacing scripts/format.sh with pre-commit (pre-commit is currently missing eslint) tested locally: <img width="898" height="929" alt="image" src="https://github.com/user-attachments/assets/58c77fb7-bdde-47ae-ac2b-b864334b3f30" /> --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> Signed-off-by: xgui <xgui@anyscale.com>
elliot-barn
added a commit
that referenced
this pull request
Oct 23, 2025
adding eslint and prettier script to precommit before getting rid of format.sh 1 step closer to replacing scripts/format.sh with pre-commit (pre-commit is currently missing eslint) tested locally: <img width="898" height="929" alt="image" src="https://github.com/user-attachments/assets/58c77fb7-bdde-47ae-ac2b-b864334b3f30" /> --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
landscapepainter
pushed a commit
to landscapepainter/ray
that referenced
this pull request
Nov 17, 2025
adding eslint and prettier script to precommit before getting rid of format.sh 1 step closer to replacing scripts/format.sh with pre-commit (pre-commit is currently missing eslint) tested locally: <img width="898" height="929" alt="image" src="https://github.com/user-attachments/assets/58c77fb7-bdde-47ae-ac2b-b864334b3f30" /> --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Aydin-ab
pushed a commit
to Aydin-ab/ray-aydin
that referenced
this pull request
Nov 19, 2025
adding eslint and prettier script to precommit before getting rid of format.sh 1 step closer to replacing scripts/format.sh with pre-commit (pre-commit is currently missing eslint) tested locally: <img width="898" height="929" alt="image" src="https://github.com/user-attachments/assets/58c77fb7-bdde-47ae-ac2b-b864334b3f30" /> --------- Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com> Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
adding eslint and prettier script to precommit before getting rid of format.sh
1 step closer to replacing scripts/format.sh with pre-commit (pre-commit is currently missing eslint)
tested locally:
