Open
Conversation
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
Member
|
Tried running this locally, but it currently fails on arm64, because there is no arm64 variant for Error: choosing an image from manifest list docker://becheran/mlc:latest: no image found in image index for architecture "arm64", variant "v8", OS "linux" |
maflcko
reviewed
Feb 10, 2026
b63444e to
1ffbea7
Compare
Member
Author
This should be fixed in the latest push. We currently (and still here) use an x86_64 MLC binary, but this works in an arm64 container (e.g on Mac) because the Rosetta emulation is passed in/used inside the container. I keep this behaviour for now, but ideally MLC woudl start publishing multi-platform docker images. |
This was referenced Feb 12, 2026
1ffbea7 to
002b15b
Compare
ad41339 to
dcef851
Compare
https://docs.astral.sh/uv/ Install python in the linter using uv and a venv. This is faster and more simple than building pyenv.
dcef851 to
f9cd96f
Compare
Align with uv tool installation method.
f9cd96f to
c7e4a67
Compare
Member
Author
|
bumped to latest |
maflcko
reviewed
Mar 13, 2026
Member
There was a problem hiding this comment.
left some comments, but feel free to ignore them
c7e4a67~8 🥝
Show signature
Signature:
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: c7e4a67086626b8832372cf461cee30c22c62ea0~8 🥝
IVhYiz19o3yQe68zl8z74QmB+pkeGtEnNKMDk40qNOxOuO20cBwlEWe2l+juySUkL1C+N5ejQa0svHEqvOa2BQ==
Add ty typechecker configuration file. Includes all needed rules to have `ty` pass on selected files in current codebase. This is pretty much all useful rules, but we will remove them systematically in follow-ups.
Remove mypy and switch to `ty` for typechecking. https://docs.astral.sh/ty/ https://astral.sh/blog/ty It's faster and more maintained than mypy. Comes with better editor integrations, a language server and is written in rust (tm). As features are added we may be able to replace vulture with ty too. Install from docker image as per `uv` and `ruff`.
Note: we keep MLC install via curl binary download. This is currently hardcoded to an x86_64 binary, but this seems to run fine in an arm64 container (on MacOS), as Rosetta support is passed in (on docker and podman) to take care of exactly this. If upstream MLC start providing a multi-image dockerfile then this dependency could be made it's own `FROM --...` layer as most of the others are.
.python-version always matches the minimum supported Python version. It's main purpose is to catch accidental use of too modern syntax in scripts and functional tests. We (currently) don't specify a minimum patch version, so it's not necessary to do so here. The minor verion is enough. This also avoids requiring users to keep a potentially unsafe old patch version installed.
Enforce monotonic ruff formatting for Python files: if a file was already ruff format-compliant on the base branch, it must stay formatted in the PR. Files that aren't yet formatted are not enforced, allowing incremental adoption without a big-bang reformat. This follows the strategy used by nixpkgs to adopt nixfmt without merge queues: 1. Add per-file CI check (this commit) — a no-op today since no files are ruff format-compliant yet. 2. Reformat inactive files (files not touched by recent PRs) in a batch. The check prevents regressions from that point on. 3. Reformat remaining files once old PRs have flushed through. The per-file approach avoids the merge queue requirement: old PRs opened before a batch reformat can still merge without breaking master, because the check only compares against the base branch version of each changed file, not the entire tree.
c7e4a67 to
2464142
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Modernise our lint tooling by:
- Replacing pyenv + pip with uv for better Python environment and dependency management
- Replacing mypy with https://github.com/astral-sh/ty
- Replacing the 01_install.sh runtime install script with COPY --from multi-stage Docker image imports for uv, ruff, shellcheck, mlc, and ty
- Moving ruff lint rules from hardcoded Rust array (in lint_py.rs) into a top-level ruff.toml, and add ty.toml for the type checker
- Extracting all remaining pip dependencies into dedicated ci/lint/requirements.txt
- Add a ruff format lint which, if the original file is formatted, will fail if a change un-formats it
Extra rationale:
COPY --frompulls pre-built binaries from upstream images instead of compiling/downloading at runtime. Containerfile layer optimisations reduce rebuild frequency further.tyis significantly faster/more modern/maintained thanmypy, and configured declaratively.Adding root-level
[ty|ruff].tomlconfig files means contributors can easily runty checkorruff checklocally without running the full linter, along with being accessible to other tooling (similarly for requriements.txt).Pinning tool versions in the dockerfile makes it more excplicit and easier to find.
The tradeoff we make here is that there is no longer a bare install script to install tooling on a local machine. However I think this is OK, as it currently only works for
apt-based OSes anyway, and I don't think running the linter outside of the container is such a valuable use-case as it is with some of the other CI jobs.Further work can drop individual rules from
ty.tomlfixing up the infringing code as necessary.I can split this up if wanted, but IMO it makes sense to do it altogether.