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.

Sign up for GitHub

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

Jump to bottom

Clippy cleanup #818

Merged
merged 34 commits into from
Dec 20, 2017
Merged

Clippy cleanup #818

merged 34 commits into from
Dec 20, 2017

Conversation

Susurrus
Copy link
Contributor

Various fixes for errors and warnings pointed out by clippy.

@Susurrus
Copy link
Contributor Author

@asomers I think there's only one controversial change in this PR, 7d733d4. Based on the comments I think these should have been const rather than standard let variables.

@Susurrus
Copy link
Contributor Author

@asomers I'd like to merge this, any chance to look over the commit I mentioned above?

@asomers
Copy link
Member

asomers commented Dec 19, 2017

I should be able to get to it sometime tonight.

@asomers
Copy link
Member

asomers commented Dec 20, 2017

I think the reason for the 'static declarations that you removed in 7d733d4 was because originally those variables were const instead of let. And as you say in one of your other commits, constants had to be explictly 'static until 1.17.0. I'm not sure why I removed the const before I committed.

@asomers
Copy link
Member

asomers commented Dec 20, 2017

The commit message for 263ed2c is incorrect. It should say "elide" or "remove implied" instead of "remove elided".

@Susurrus
Copy link
Contributor Author

@asomers The tests still seem to pass, do you think the tests are correct to begin with?

@asomers
Copy link
Member

asomers commented Dec 20, 2017

Once I finish converting aio from using Rc<[u8]> to Bytes, a81dba7 will be completely irrelevant. But it's fine to commit it anyway.

@asomers
Copy link
Member

asomers commented Dec 20, 2017

Yes, the tests are correct. They don't rely on wbuf being const.

@asomers
Copy link
Member

asomers commented Dec 20, 2017

7f3ae6b and 5568128 are the same thing. It would make sense to squash those two together.

@asomers
Copy link
Member

asomers commented Dec 20, 2017

Apart from that one commit message, everything looks acceptable to me. Just fix that, then bors +asomers .

@Susurrus
Copy link
Contributor Author

Awesome, thanks @asomers! I merged those two commits and gave that other one a better title.

Let's wait to merge this until #648 is merged as I really want to get that one in. It's the last of the FFI issues we have in nix, been a long time trying to knock all of those out.

Bryant Mairs added 17 commits December 20, 2017 07:05
Use write_all instead of write
149d6c1 
Several tests make the assumption that all data is written, which
is not guaranteed with write(), so use write_all() instead.
Use byte strings instead of String.as_bytes()
73ea706 
A little easier to read
Add backticks to types in doccomments
68859b9
Remove useless vec![]
09a79fa
Remove elided 'static lifetime
ae8e4a4 
As of Rust 1.17 'static lifetimes are implied when
declaring consts.
Remove 'static from variables
e831466 
It's unclear why these were static in the first place.
Use iter::Cloned() instead of closure
070c017
Remove unnecessary .ok()
4bad9bf
Make numeric literals easier to read
d8c5385
Use println!() instead of print!(...n)
1c9f6c8
Remove unnecessary references
b93c1ad
Use Rc::clone() instead of .clone()
7b4402f 
Makes it more clear what's being cloned
Remove useless Err(_) match arms
46fe131
Correct ioctl read and write_ptr tests
ee75274 
Looks like a copy/paste error might have caused this
Remove unnecessary mut
a6b394e
Use '!' instead of '== false'
b814db0 
Reads a little bit easier
Remove match statements with one arm
d203e92
Bryant Mairs added 17 commits December 20, 2017 07:05
@Susurrus
Copy link
Contributor Author

bors r+ asomers

bors bot added a commit that referenced this pull request Dec 20, 2017
@bors
Merge #818
dad7770 
818: Clippy cleanup r=Susurrus a=Susurrus

Various fixes for errors and warnings pointed out by clippy.
@bors
Copy link
Contributor

bors bot commented Dec 20, 2017

Build succeeded

@bors bors bot merged commit 8db68be into nix-rust:master Dec 20, 2017
@Susurrus Susurrus deleted the clippy_cleanup branch December 21, 2017 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Susurrus @asomers