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

style: remove unnecessary mut and unsafe keywords #84

Merged
merged 3 commits into from
Apr 19, 2018

Conversation

oherrala
Copy link
Contributor

@oherrala oherrala commented Apr 6, 2018

Compiler nagged about these. Remove it to please the compiler.

Compiler nagged about these. Remove it to please the compiler.
@indiv0
Copy link
Owner

indiv0 commented Apr 10, 2018

Apparently this changed in a recent compiler version, so the Travis test for rustc 1.13.0 is failing. Would you mind figuring out the minimum rustc version on which this PR will compile and update the .travis.yml accordingly?

Also, please add yourself to CONTRIBUTORS.md in the PR.

@indiv0 indiv0 requested review from indiv0 and removed request for indiv0 April 10, 2018 23:43
@indiv0 indiv0 self-assigned this Apr 10, 2018
@oherrala
Copy link
Contributor Author

Sorry, I didn't get any message from Travis about the fail.

Apparently change required for this PR is released in Rust 1.25.0. Here are related issue and PR: rust-lang/rust#35067 and rust-lang/rust#47204.

This change is so minor that breaking compatibility with older Rust is not beneficial so this change could wait and instead add #[allow(unused_unsafe)] unsafe { ... } to remove the nag for newer compilers.

@indiv0
Copy link
Owner

indiv0 commented Apr 11, 2018

Great! Looks good to me. Minor nitpicks though, would you mind amending the commits by adding style: and chore: prefixes to their messages and removing the capitalization on "Allow" and "Update" (just so that my changelog generator doesn't complain).

Rust 1.25 changed UnsafeCell::into_inner() from unsafe to safe
function. This unsafe can be removed when supporting Rust older than
1.25 is not needed.
@oherrala
Copy link
Contributor Author

Re-pushed changes with fixed commit messages.

@indiv0 indiv0 merged commit 343cf44 into indiv0:master Apr 19, 2018
@indiv0
Copy link
Owner

indiv0 commented Apr 19, 2018

Awesome, thank you for the PR!

@oherrala oherrala deleted the fix-nags branch April 19, 2018 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants