Skip to content

Conversation

MichaReiser
Copy link
Member

Summary

Upgrade to Rust 1.73

Test Plan

cargo test

@MichaReiser
Copy link
Member Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@@ -149,7 +149,7 @@ impl Notebook {
{
let trailing_newline = reader.seek(SeekFrom::End(-1)).is_ok_and(|_| {
let mut buf = [0; 1];
reader.read_exact(&mut buf).is_ok_and(|_| buf[0] == b'\n')
reader.read_exact(&mut buf).is_ok_and(|()| buf[0] == b'\n')
Copy link
Member

Choose a reason for hiding this comment

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

Not convinced by that clippy change tbh (the other ones are good though)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't mind disabling that lint... Here's the documentation of the rule and why https://rust-lang.github.io/rust-clippy/master/index.html#/ignored_unit_patterns

I was like, meh but then it's rare and not too bad

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with it, don't feel strongly.

@MichaReiser MichaReiser added the internal An internal refactor or improvement label Oct 17, 2023
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 17, 2023

CodSpeed Performance Report

Merging #8007 will improve performances by 4.99%

Comparing rust-1.73.0 (acc594f) with main (d6a4283)

Summary

⚡ 3 improvements
✅ 22 untouched benchmarks

Benchmarks breakdown

Benchmark main rust-1.73.0 Change
lexer[pydantic/types.py] 4 ms 3.8 ms +4.62%
lexer[unicode/pypinyin.py] 583.1 µs 559 µs +4.3%
lexer[large/dataset.py] 8.9 ms 8.5 ms +4.99%

@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

@charliermarsh
Copy link
Member

I'd been holding off on this as Homebrew doesn't yet support it (conda-forge does): Homebrew/homebrew-core#149754

@MichaReiser
Copy link
Member Author

MichaReiser commented Oct 23, 2023

I'd been holding off on this as Homebrew doesn't yet support it (conda-forge does): Homebrew/homebrew-core#149754

This should be fine as far as I understand because I only bumped our toolchain version but not the minimal required Rust version (Cargo.toml where we still use 1.71)

@MichaReiser MichaReiser enabled auto-merge (squash) October 23, 2023 01:59
@MichaReiser MichaReiser merged commit 2c2ebf9 into main Oct 23, 2023
@MichaReiser MichaReiser deleted the rust-1.73.0 branch October 23, 2023 02:12
@MichaReiser
Copy link
Member Author

Just tested brew install ruff --head and it compiles fine :)

@chenrui333
Copy link

FYI, rust 1.74.0 got shipped in the homebrew side now, Homebrew/homebrew-core#154526

@charliermarsh
Copy link
Member

Thank you @chenrui333!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants