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

Miscellaneous updates #111

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Miscellaneous updates #111

wants to merge 11 commits into from

Conversation

lrazovic
Copy link

  • Use Rust 2018 Edition, this bumps MSRV to 1.31.0.
  • Use the include filed in Cargo.toml to reduce the crate size by 10%.
  • Update libc to latest version.
  • Update hermit-abi to latest version.
  • Use Self instead of structs name in the impls
  • Use MaybeUninit instead of mem::zeroed. This removes an unsafe block, but bumps MSRV to 1.36.0.
  • Made Cgroup::new() a const function, this bumps MSRV to 1.32.0.

I understand that it's a lot of changes, and especially for those that increase the level, I'm ready to discuss its implementation and in case go back.

I tested this PR on Linux and Windows, using cargo test --all-features --all-targets, miri and clippy.

@vilgotf
Copy link

vilgotf commented Sep 29, 2021

  • Update libc to latest version.

This just disallows older libc versions for no reason. The latest libc version is already allowed (libraries don't generally specify max versions in rust, the specific version is decided by applications in Cargo.lock)

@lrazovic
Copy link
Author

Thank you so much for the explanation, now I learned something new. I removed the dependency updates from the PR.

@seanmonstar
Copy link
Owner

Sorry for missing this 🙈

The dependency change is a good catch, we should stay conservative. Also, any changes that increase the MSRV should probably only be done because of a huge improvement. With such a basic, fundamental crate, we don't want to make it so people couldn't upgrade (even if they are using some ancient compiler).

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.

None yet

3 participants