Skip to content

ci: fix MSRV check#5799

Merged
wjones127 merged 6 commits intolance-format:mainfrom
wjones127:ci/msrv-check
Jan 24, 2026
Merged

ci: fix MSRV check#5799
wjones127 merged 6 commits intolance-format:mainfrom
wjones127:ci/msrv-check

Conversation

@wjones127
Copy link
Copy Markdown
Contributor

@wjones127 wjones127 commented Jan 23, 2026

The rust-toolchain.toml file has been making our current MSRV check inert. This environment variable should override it.

Also updates MSRV to 1.88 (same as DataFusion) and makes associated clippy fixes.

@github-actions github-actions Bot added python ci Github Action or Test issues java labels Jan 23, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Code Review

P0 - YAML Syntax Error: The env syntax on line 273 is incorrect. GitHub Actions expects env: as a mapping (with a colon and newline), not env: KEY=VALUE on the same line.

The current change:

      - name: cargo +${{ matrix.msrv }} check
        env: RUSTUP_TOOLCHAIN=${{ matrix.msrv }}

Should be:

      - name: cargo +${{ matrix.msrv }} check
        env:
          RUSTUP_TOOLCHAIN: ${{ matrix.msrv }}

The idea to use RUSTUP_TOOLCHAIN instead of rustup default is correct - this environment variable properly overrides the rust-toolchain.toml file. However, the current YAML syntax will cause the workflow to fail to parse.


The MSRV updates in java/lance-jni/Cargo.toml and python/Cargo.toml (1.80 → 1.82) correctly align with the workspace rust-version = "1.82.0" in the root Cargo.toml.

Comment on lines +272 to +273
env:
RUSTUP_TOOLCHAIN: ${{ matrix.msrv }}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This tells it to ignore the toolchain file basically?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah it's higher up in the resolution chain: https://rust-lang.github.io/rustup/overrides.html

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 23, 2026

Codecov Report

❌ Patch coverage is 81.25000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ncoding/src/previous/encodings/physical/bitpack.rs 0.00% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@wjones127 wjones127 marked this pull request as ready for review January 24, 2026 01:34
@wjones127 wjones127 merged commit a902892 into lance-format:main Jan 24, 2026
26 of 27 checks passed
vivek-bharathan pushed a commit to vivek-bharathan/lance that referenced this pull request Feb 2, 2026
The `rust-toolchain.toml` file has been making our current MSRV check
inert. This environment variable should override it.

Also updates MSRV to 1.88 (same as DataFusion) and makes associated
clippy fixes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Github Action or Test issues java python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants