Skip to content

feat: add lint and fix commands to the Makefile#10

Closed
moliholy wants to merge 3 commits intomasterfrom
feat/lint-fix
Closed

feat: add lint and fix commands to the Makefile#10
moliholy wants to merge 3 commits intomasterfrom
feat/lint-fix

Conversation

@moliholy
Copy link
Member

@moliholy moliholy commented Jan 30, 2026

Summary

Add make lint and make fix commands that emulate the CI checks indico core follows, scoped to only changed files in the indico submodule. Covers Python (isort, unbehead, ruff) and JS/CSS (biome, eslint, tsc, stylelint) tooling.

Also adds the virtualenv to PATH via direnv so that indico's pre-commit hook can find the required binaries (ruff, python, isort).

@moliholy moliholy requested a review from OmeGak January 30, 2026 11:16
@moliholy moliholy self-assigned this Jan 30, 2026
@moliholy
Copy link
Member Author

moliholy commented Feb 8, 2026

@OmeGak friendly ping.

@moliholy moliholy changed the title feat: lint and fix commands feat: add lint and fix commands to the Makefile Feb 8, 2026
Ensures ruff, python, isort and other venv binaries are available
directly, which is needed by indico's pre-commit hook.
Copy link
Member

@OmeGak OmeGak left a comment

Choose a reason for hiding this comment

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

I'm really not sure that this repository should have Makefile targets for the many different actions that we may want to run in Indico core or any of the plugins.

I have made my arguments in the PR from @duartegalvao that adds pylint to the repo (https://github.com/unconventionaldotdev/indicorp/pull/11/changes#r2782634229).

Each plugin directory is its own standalone project with its own architecture and QA decisions. This Makefile target breaks that isolation in ways that I'm not sure we should. If we did, then, why not also adding a target for building wheels, linting, another one for DB schema migrations, etc.?

I'm checking with Adrian if we can add a Makefile in Indico core. If we had it, the workflow would be:

cd indico
make lint

Which I think is very reasonable.

Comment on lines +3 to +4
# Make virtualenv binaries (ruff, python, isort, etc.) available on PATH
PATH_add .venv/bin
Copy link
Member

Choose a reason for hiding this comment

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

Rather than mangling the PATH, I would prefer that we rely on uv run.

Copy link
Member Author

@moliholy moliholy Feb 9, 2026

Choose a reason for hiding this comment

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

Then you have to run uv run git commit, because pre-commit hooks are triggered. I do not recommend that.

Copy link
Member

Choose a reason for hiding this comment

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

The pre-commit hook assumes that the virtualenv is active.

@moliholy
Copy link
Member Author

moliholy commented Feb 9, 2026

I'm checking with Adrian if we can add a Makefile in Indico core. If we had it, the workflow would be:

If so, fine for me. But still, until that's done it doesn't hurt to have our own and then remove it. Not a string opinion at all either.

@OmeGak
Copy link
Member

OmeGak commented Feb 9, 2026

@ThiefMaster agreed with having a Makefile in Indico core with linting and test targets following a logic similar to this:

.PHONY: lint
lint: lint-py lint-js lint-locales lint-headers

.PHONY: lint-py
lint-py: lint-py-isort lint-py-ruff lint-py-backrefs

.PHONY: lint-py-isort
lint-py-isort:
	isort --check-only

.PHONY: lint-py-ruff
lint-py-ruff:
	ruff check --output-format=concise .

.PHONY: lint-py-backrefs
lint-py-backrefs:
	bin/maintenance/update_backrefs.py --ci

.PHONY: lint-js
lint-js:
	...

.PHONY: lint-locales
lint-locales:
	bin/maintenance/update_moment_locales.py --ci

.PHONY: lint-icons
lint-icons:
	bin/maintenance/generate_icons.py --ci

.PHONY: lint-headers
lint-headers:
	unbehead --check

@OmeGak OmeGak closed this Feb 12, 2026
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