Skip to content

[onhold] chore: setup frontend pre commit hook #1161

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.

Sign up for GitHub

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

Draft
khaledosman wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

[onhold] chore: setup frontend pre commit hook #1161

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5b2f883
setup husky & lint-staged
khaledosman Mar 10, 2025
2574e89
unset hooksPath before running pre-commit install
khaledosman Mar 10, 2025
771664b
remove --all-files
khaledosman Mar 10, 2025
308a473
reinstall husky after uv pre-commit
khaledosman Mar 10, 2025
6fea93f
Merge branch 'main' into frontend-pre-commit
khaledosman Mar 11, 2025
eafa81a
Merge branch 'main' into frontend-pre-commit
khaledosman Mar 12, 2025
4ce0d50
remove unnecessary uv pre-commit install
khaledosman Mar 12, 2025
b274e83
Merge branch 'main' into frontend-pre-commit
khaledosman Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ endef

# Launches Lumigator in 'development' mode (all services running locally, code mounted in)
local-up: config-generate-env
git config --unset-all core.hooksPath
uv run pre-commit install
pushd lumigator/frontend && npm run prepare && popd
Copy link
Contributor Author

@khaledosman khaledosman Mar 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

when running npm install it will now run the prepare script in

lumigator/lumigator/frontend/package.json

Line 18 in 2574e89

"prepare": "cd ../../ && husky install lumigator/frontend/.husky"

husky sets the core.hooksPath to frontend/.husky during its installation which breaks whatever checks uv pre-commit does when running make local-up (core.hooksPath defines the folder/path where github looks at to see where the hooks are defined)

I was getting this error when running make local-up

uv run pre-commit install
[ERROR] Cowardly refusing to install hooks with core.hooksPath set.
hint: git config --unset-all core.hooksPath
make: *** [local-up] Error 1

so this is a workaround to unset core.hooksPath before running uv run pre-commit install to make things work then run the npm prepare script to install husky (which sets git config core.hooksPath to frontend/.husky) and make the frontend pre-commit hook work alongside the backend one, where we run the uv run pre-commit run from frontend/.husky/pre-commit (so technically I think we can even get rid of running uv run pre-commit install in the make file since its already part of the pre-commit hook in .husky)

This feels hacky, but I'm not sure if there's a better way to do it. cc @macaab26, essentially all the frontend needs to do is run npx lint-staged from the frontend folder, so to avoid all this setting/unsetting hooksPath stuff, we could technically get rid of husky and rely on the normal .git/hooks/pre-commit however this file is automatically generated/overwritten by ruff so that wouldn't work either, perhaps we could create a custom pre-commit hook file and manually append to the generated pre-commit hooks or something along these lines which would also kinda hacky 🤷, or we could just rely on husky to run both backend & frontend linting which is what it currently does

Copy link
Contributor Author

@khaledosman khaledosman Mar 12, 2025

Choose a reason for hiding this comment

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

removed uv run pre-commit install in 4ce0d50 now that .husky/pre-commit is responsible for running linting for both backend and frontend where it would run uv run pre-commit run and npx lint-staged

RAY_ARCH_SUFFIX=$(RAY_ARCH_SUFFIX) ARCH=${ARCH} COMPUTE_TYPE=$(COMPUTE_TYPE) docker compose --env-file "$(CONFIG_BUILD_DIR)/.env" --profile local $(GPU_COMPOSE) $(MODEL_CACHE_COMPOSE) -f $(LOCAL_DOCKERCOMPOSE_FILE) -f $(DEV_DOCKER_COMPOSE_FILE) up --watch --build

local-down: config-generate-env
Expand Down
15 changes: 15 additions & 0 deletions lumigator/frontend/.husky/pre-commit
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

# FRONTEND_DIR="$(dirname "$0")/../lumigator/frontend"

# Run backend linting with pre-commit
echo "Running backend linting..."
uv run pre-commit run

# Run frontend linting with lint-staged
echo "Running frontend lint-staged..."
cd lumigator/frontend
npx lint-staged

echo "✅ Pre-commit checks passed!"
Loading