Skip to content

Conversation

rivy
Copy link
Member

@rivy rivy commented Dec 30, 2019

This is a multi-week effort to fix all cargo clippy lint complaints.

I've made significant effort to segregate the changes by clippy category per commit.

The last commits (containing "allow ...") are added allow attributes for code that clippy doesn't like but has no easy refactor (or would increase MinSRV).

I tried to be very mechanical and straight-forward about all refactors. But, I'm happy to work on any of this if you disagree with any of the refactoring choices or the "allow" commits.

  • note: this PR is on top of the "fix+modernize" PR in order to be able to easily build and test on multiple machines

@rivy rivy mentioned this pull request Dec 30, 2019
@rivy
Copy link
Member Author

rivy commented Dec 30, 2019

All tests pass, see AppVeyor for 13fb466c; AppVeyor fail here is some transient clone error. A restart would probably fix it.

@ArniDagur
Copy link
Contributor

In my opinion we should update the Minimum Supported Release to 1.36, and fix the uses of mem::uninitialized properly. What do you think @Arcterus?

@cnd cnd requested a review from Arcterus January 14, 2020 07:39
@Arcterus
Copy link
Collaborator

Sorry for lack of response. I will be more active going forward.

@ArniDagur I agree. I'm actually tempted to bump us up to 1.39 to experiment with async/await.

@rivy can you also rebase? The only thing that should be different so far is uptime. In the meantime I'll start looking through the changes over the next day or two. Would you prefer I merge the other two PRs first or just merge this one when it's done?

rivy added 24 commits January 27, 2020 21:33
- convert to newer `..=` syntax, fixing compiler warnings

+ requires MinSRV >= v1.26.0

.# [why]

The inclusive range operator (`..=`) was stabilized in rust v1.26.0.

Warnings requesting conversion from the old `...` operator to `..=` were
introduced in rust v1.37.0.

* ref: <https://github.com/rust-lang/rust/blob/master/RELEASES.md>
- convert to newer `?` syntax, fixing compiler warnings

+ requires MinSRV >= v1.13.0

.# [why]

The `?` operator was stabilized in rust v1.13.0.

Warnings requesting conversion from the old `try!` macro to the `?` operator
were introduced in rust v1.39.0.

* ref: <https://github.com/rust-lang/rust/blob/master/RELEASES.md>
…t,end}...`

- `trim_{start,end}{,_matches}` stabilized in rust v1.30.0
- `trim_{start,end}{,_matches}` deprecated (with compiler warnings) in rust v1.33.0

+ requires MinSRV >= v1.30.0

* ref: https://github.com/rust-lang/rust/blob/master/RELEASES.md
+ ToDO added ~ when possible, test under WSL2 and differentiate/liberalize if possible
…ions

- allow actual outputs to differ from expected (ie, `stat`) if `stat` is reporting "unknown" creation time

.# [why]

For many *nix flavors, `stat` is unable to detect birth/creation date
for directories/files. The information is available via the `statx()`
system call (for linux kernels >= v4.11), and rust supplies that
information via fs::MetadataExt for v1.40+. So, for rust v1.40+, there
will likely be a mismatch between the output of the system `stat` and
this ('uutils') `stat`.

* ref: <https://askubuntu.com/questions/470134/how-do-i-find-the-creation-time-of-a-file> @@ <https://archive.is/IsEAJ>
.# [why]

`mem::ununitialized()` is deprecated as of rust v1.39.0.

And this use is likely a premature optimization attempting to avoid
initializing the byte array to 0, which is usually a *very* fast operation.

* ref: <https://github.com/rust-lang/rust/blob/master/RELEASES.md>
* ref: <https://stackoverflow.com/questions/3654905/faster-way-to-zero-memory-than-with-memset>
rivy added 22 commits January 27, 2020 22:35
.# [why]

Although `copied()` is preffered, it was not stabilized until rust v1.35.0.
So, 'map_clone' is allowed instead of increasing MinSRV to v1.35.0.

* ref: https://github.com/rust-lang/rust/blob/master/RELEASES.md
…::uninitialized)

.# [why]

`std::mem::MaybeUninit` is likely preffered. But `MaybeUninit` was not
stabilized until rust v1.36.0 and conversion from `mem::uninitialized` is
not obviously straight-forward at the moment.

So, 'std::mem::uninitialized' is allowed instead of increasing MinSRV to v1.36.0.

* ref: https://github.com/rust-lang/rust/blob/master/RELEASES.md
@rivy
Copy link
Member Author

rivy commented Jan 28, 2020

@Arcterus , glad to see you back!

I've rebased all three PRs and committed a couple of needed fixups.

I think merging the sub-PRs first would be easier to digest, but I'm happy to support whatever is easier for you.

I've got one more substantial PR that I'll post tomorrow which includes refactored features so that cargo build just works on all platforms (compiling the common subset of utils), a more robust cargo make bundle that tailors the build to the platform automatically and can build individual utilities easily. And, the best part... is a full GitHub Action CICD that performs about 5 to 10 times faster that Travis or AppVeyor. And it can do automatic release deployment for tagged commits.

@rivy rivy mentioned this pull request Jan 29, 2020
@rivy
Copy link
Member Author

rivy commented Apr 7, 2020

Closing in favor of #1449 (which includes these commits).

@rivy rivy closed this Apr 7, 2020
@rivy rivy deleted the fix.lints branch April 7, 2020 23:10
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.

3 participants