Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minimum supported rust version policy #668

Closed
4 tasks done
clux opened this issue Oct 24, 2021 · 5 comments · Fixed by #687
Closed
4 tasks done

minimum supported rust version policy #668

clux opened this issue Oct 24, 2021 · 5 comments · Fixed by #687
Assignees
Labels
automation ci and testing related docs unclear documentation

Comments

@clux
Copy link
Member

clux commented Oct 24, 2021

Moving towards stability land (#508), we should probably could do a best effort to maintain builds on the last few stable versions of rust. (Originally discussed briefly in #665).

Problems:

  • need to restrict dependency updates to upstream policies of their MSRV (which is hard with our dependency tree, but presumably it is automated if people set the rust-version)
  • need CI to validate that our MSRV is building on that version
  • people suggest a 6 month policy, but it's undoubtedly going to put a strain on maintainers

We just merged in edition 2021 so there's limits to how much we can do (and the 2021 release is incidentally the release that supports rust-version key. For now we could follow this general plan:

and then maybe in the futere (depending on need, and what type of problems we run into):

  • speculatively commit to 3 months initially (that's 2 stable rust releases back)
  • write a speculative policy document somewhere (e.g. something like wg/msrv)

but that will be one for a later issue - for now; our policy is ad-hoc; but documented in the readme and cargo metadata.

@clux clux added the automation ci and testing related label Oct 24, 2021
clux added a commit that referenced this issue Oct 24, 2021
Signed-off-by: clux <sszynrae@gmail.com>
@clux
Copy link
Member Author

clux commented Oct 24, 2021

edited a bit because not sure we should do this stuff yet. waiting for some more feedback.
have added the rust-version key at least though. that seems like good practice.

@clux clux added the question Direction unclear; possibly a bug, possibly could be improved. label Oct 24, 2021
@kazk
Copy link
Member

kazk commented Oct 25, 2021

I don't know what's best for kube yet (trade-offs), but adding a specific version to CI is a step forward (MSRV discussion in api-guidelines: rust-lang/api-guidelines#123 (comment)). The new rust-version field resolves the pain of finding MSRV of dependencies discussed there, and I'm sure we'll have tools to help in the future as more crates set this field.

The oldest rust-version kube can set is the newest rust-version from the dependencies. Likewise, kube's rust-version may determine the oldest for the dependents. So, in general, lower level crates should have a longer MSRV policy. 3 months for kube sounds decent to me, and that can be a best effort basis before v1 (at least consider if bumping rust-version is worth the gain).

Personally, I wouldn't mind kube having rust-version set to the latest stable at the time of each release, but that's because I don't have any use not under my control. Maintainers of popular tools/libraries will appreciate this.

  • add CI to run against 1.56 (with a sanity check that the rust version matches what we set in Cargo.tomls
  • run that ci job daily

Can't we add a new job msrv that checks if it compiles with rust-version? Few jobs run in parallel, so CI shouldn't take that much additional time. Not sure if we need to run all the tests for MSRV check.

@lfrancke
Copy link
Contributor

lfrancke commented Oct 26, 2021

FWIW: It'd be nice to have the current MSRV in the readme. Yes, I know, I can read the policy and I can look at a cargo.toml but if I come in through Github that's the first thing I see.

I'm also fine with 3 months and in general everything you and @kazk said sounds fine to me. Thank you!

@kazk
Copy link
Member

kazk commented Oct 26, 2021

We can add a badge.

Rust 1.56

#dea584 is the color GitHub uses for Rust.

@clux
Copy link
Member Author

clux commented Oct 26, 2021

That badge is great. I wrote a small script to generate that from cargo-msrv.

@clux clux moved this to Defining in Kube Roadmap Oct 26, 2021
@clux clux moved this from Defining to In Progress in Kube Roadmap Oct 30, 2021
@clux clux self-assigned this Oct 30, 2021
@clux clux removed the question Direction unclear; possibly a bug, possibly could be improved. label Oct 30, 2021
@clux clux linked a pull request Oct 31, 2021 that will close this issue
@clux clux moved this from In Progress to Blocked in Kube Roadmap Nov 3, 2021
@clux clux added the docs unclear documentation label Nov 7, 2021
@clux clux closed this as completed in #687 Nov 20, 2021
clux added a commit that referenced this issue Nov 20, 2021
* check msrv on CI

Signed-off-by: clux <sszynrae@gmail.com>

* determine MSRV from readme

Signed-off-by: clux <sszynrae@gmail.com>

* ok fine use grep

Signed-off-by: clux <sszynrae@gmail.com>

* can cache this

Signed-off-by: clux <sszynrae@gmail.com>
Repository owner moved this from Blocked to Done in Kube Roadmap Nov 20, 2021
@clux clux mentioned this issue Nov 21, 2021
33 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation ci and testing related docs unclear documentation
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants