Skip to content

ci: add black + flake8 lint check workflow#2215

Open
genisis0x wants to merge 1 commit into
microsoft:mainfrom
genisis0x:feat/lint-check-ci-2060
Open

ci: add black + flake8 lint check workflow#2215
genisis0x wants to merge 1 commit into
microsoft:mainfrom
genisis0x:feat/lint-check-ci-2060

Conversation

@genisis0x
Copy link
Copy Markdown

Summary

Fixes #2060.

Adds a `Lint check` GitHub Actions workflow that runs the existing `make black` and `make flake8` targets on every push to `main` and every pull request. PRs surface formatting and style regressions at review time instead of relying on contributors to remember to run the local Makefile targets first.

Confirmed locally that both targets are clean against current `main` (`231 files would be left unchanged` for black, no output for flake8), so the workflow will be green on day one.

Why this deviates from the issue's sample

The issue's sample workflow pushed auto-fix commits back to the PR branch. I intentionally did not include that for one reason:

  • Auto-commit on a PR requires `pull_request_target` + `contents: write`. That combination runs with repo-write secrets in the context of an external fork's code, which is the canonical fork-PR privilege-escalation vector. Maintainers would (rightly) need to vet every PR before letting that workflow run, which defeats the "automated" part.

A read-only check gives the same lint-enforcement benefit with no fork-PR security tradeoff; contributors run `make black` / `make flake8` locally to fix issues. Happy to add the auto-fix variant in a separate workflow guarded by `workflow_dispatch` or `pull_request` (not `_target`) if maintainers prefer that direction.

Why only black + flake8, not all of `make lint`

`make lint` chains `black`, `pylint`, `flake8`, `mypy` and `nbqa`. The Makefile shows long disable-lists for pylint and mypy carrying historical warnings the team has left intentionally unaddressed. Folding those into a hard gate would either fail the workflow on every PR or require maintainers to first burn down the backlog. Keeping the initial CI binary on the two strict tools means PRs land green or red without a noisy yellow channel; the heavier checks can be folded in via follow-up once maintainers decide what threshold they want there.

Files

  • `.github/workflows/lint.yml` — new, 53 lines.

Validation

```
$ make black
black . -l 120 --check --diff --exclude qlib/_version.py
All done! ✨ 🍰 ✨
231 files would be left unchanged.

$ make flake8
flake8 --ignore=E501,F541,E266,E402,W503,E731,E203 --per-file-ignores="init.py:F401,F403" qlib
(no output)
```

The workflow uses Python 3.10 — middle of the supported range from `test_qlib_from_source.yml` — so the lint job stays fast and doesn't re-run identical lint over the full test matrix.

Fixes #2060

Fixes microsoft#2060. Adds a `Lint check` GitHub Actions workflow that runs the
existing `make black` and `make flake8` targets on every push to `main`
and every pull request. PRs surface formatting and style regressions at
review time instead of relying on contributors to remember to run the
local Makefile targets first.

Intentional choices:

- Only `black` and `flake8` are enforced in this initial workflow.
  `make lint` also chains `pylint`, `mypy` and `nbqa`, which have a
  larger surface and historically carry warnings the team has left
  intentionally unaddressed (see the long disable-lists in the Makefile
  comment). Keeping the CI signal binary on the two strict tools means
  PRs land green or red without a noisy yellow channel; the heavier
  checks can be folded in via follow-up if maintainers want them.
- Check-only, not auto-fix. The issue's sample workflow pushed
  generated fixup commits back to the PR branch, which requires
  `pull_request_target` and `contents: write`. That combination runs
  with repo-write secrets in the context of an external fork's code
  and is the standard fork-PR escalation vector. A read-only check
  gives the same enforcement benefit with no fork-PR security
  tradeoff; contributors run `make black` / `make flake8` locally to
  fix issues.

Pinned to a single Python (3.10, middle of the supported range from
`test_qlib_from_source.yml`) so the lint job stays fast and doesn't
re-run identical lint over the full test matrix.
@genisis0x
Copy link
Copy Markdown
Author

Read through the CLA — all good. @microsoft-github-policy-service agree

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.

auto lint fix in CI

1 participant