Housekeeping - nothing to see here, move along #90

mati865 · 2018-08-07T21:48:09Z

std::env::home_dir() was lying about it's functionality: Deprecate std::env::home_dir and fix incorrect documentation rust-lang/rust#51656
As always: new features/bugs, bug fixes, performance improvements. It would be boring but regex gained ability to detect and use SIMD on supported machines: rust-lang/regex@e455d53
Before:

 elfx86exts target/release/cargo-install-update 
MODE64 (call)
CMOV (cmovb)
SSE1 (movups)
SSE2 (movdqu)

After:

 elfx86exts target/release/cargo-install-update
MODE64 (call)
CMOV (cmovb)
SSE1 (movups)
SSE2 (movdqu)
AVX (vxorps)
AVX2 (vpcmpeqd)
SSSE3 (pshufb)

Clippy wants to help saving those miliseconds
Rustfmt now sorts lines which begin with use keyword, there is option (disabled by default) to do the same for extern crate

I can squash commits if you want.

New regex enables SIMD on supported plaftorms.

nabijaczleweli

Thank you for doing this (and taking the time to gnome this repo when I don't've the strength to, I genuinely appreciate it a tremendous lot ❤️).

Re: squashing: unnecessary, this is really well split into self-contained chunks 👍

I'd some, mainly stylistic, nitpicks, (would feel silly to immediately revert an entire commit at merge, when I put my dumb pedantry in :Р).

nabijaczleweli · 2018-08-12T02:13:40Z

src/main.rs

@@ -59,7 +59,7 @@ fn actual_main() -> Result<(), i32> {
        if !search_res.success() {
            return Err(search_res.code().unwrap_or(-1));
        }
-        println!("");


I'm pretty sure this code pre-dates zero-arg writeln!()s :Р
Good catch 👍

nabijaczleweli · 2018-08-12T02:15:22Z

src/ops/mod.rs

@@ -340,7 +340,7 @@ impl GitRepoPackage {
            GitRepoPackage {
                name: c.get(1).unwrap().as_str().to_string(),
                url: url.into_string(),
-                branch: branch,
+                branch,


I'd actually rather this stay explicit, as it's more aesthetically pleasing, if you please.

I'll revert this change and disable the lint.

nabijaczleweli · 2018-08-12T02:18:22Z

src/options.rs

@@ -225,7 +225,7 @@ fn cargo_dir() -> (String, PathBuf) {
    }
 }

-fn existing_dir_validator(label: &str, s: String) -> Result<(), String> {
+fn existing_dir_validator(label: &str, s: &str) -> Result<(), String> {
    fs::canonicalize(&s).map(|_| ()).map_err(|_| format!("{} directory \"{}\" not found", label, s))


There's little difference here though, innit, seeing as the string will've gotten drop()ped by the end of the caller's line anyway, but good idiomaticity catch 👍

(Although, does it work with fs::canonicalise(s) here? Seems superiour, innit?)

I don't remember if I tried fs::canonicalise(s) but yeah looks like it should work.

nabijaczleweli · 2018-08-12T02:20:10Z

src/main-config.rs

@@ -1,10 +1,9 @@
 extern crate cargo_update;
 extern crate tabwriter;

-use std::io::{Write, stdout};
-use tabwriter::TabWriter;
+use std::io::{stdout, Write};
 use std::process::exit;


I'd actually rather this commit dropped, consider the width-priority ordering superiour.

Fine, I'll disable use sorting in rustfmt config.

mati865 · 2018-08-12T11:29:52Z

I'd some, mainly stylistic, nitpicks, (would feel silly to immediately revert an entire commit at merge, when I put my dumb pedantry in :Р).

That's the point of the reviews 😉
I'll address it on the Monday probably.

Maybe instead of blindly bumping minimal versions it'd better to add CI checking whether the crates in said versions are buildable rust-lang/cargo#5657?

nabijaczleweli · 2018-08-12T15:58:11Z

I'm not sure if I'm extremely dumb (yes), extremely tired (hardly), or if the way the feature's explained just doesn't click for me (probably) but I somehow don't get what it does?

mati865 · 2018-08-13T13:56:28Z

Versions declared in Cargo.toml are in fact "minimal supported versions". Cargo will automatically use latest compatible version, consider this example:

[dependencies]
foo = "1.0"

You are declaring this crate will work with versions: 1.0 >= foo < 2.0.
It's not always possible to use latests versions like Cargo does by default so we have two hypothetical environments:

Cargo uses latest foo = "1.1", everything works fine.
Cargo uses older but declared to work foo = "1.0", everything works fine.

Then you start using foo::x without noticing it was added in foo = "1.1" and Cargo.toml still points to foo = "1.0":

Cargo uses latest foo = "1.1", everything works fine.
Cargo uses older but declared to work foo = "1.0" and user gets the compilation error.

There is ongoing effort to mitigate it by adding CI job with -Z minimal-versions to make sure we are not lying about supported dependencies versions. Another less nice solution is to always declare latest versions, "if we support only the latest version we cannot be lying about earlier versions".

mati865 · 2018-08-13T20:00:01Z

Updated libgit2 should fix #92

nabijaczleweli · 2018-08-14T23:28:17Z

Released in v1.6.2.

mati865 added 2 commits August 7, 2018 23:00

Replace deprecated std::env::home_dir

8b1cdbf

Bump dependencies

d286422

New regex enables SIMD on supported plaftorms.

nabijaczleweli added question enhancement and removed question labels Aug 12, 2018

nabijaczleweli requested changes Aug 12, 2018

View reviewed changes

mati865 mentioned this pull request Aug 13, 2018

Can't install on Void Linux #92

Closed

mati865 added 2 commits August 13, 2018 21:54

Clippy knows better how to do it

766527c

Do not reorder imports

9109bdf

mati865 force-pushed the master branch from 507fe76 to 9109bdf Compare August 13, 2018 19:58

nabijaczleweli approved these changes Aug 14, 2018

View reviewed changes

nabijaczleweli merged commit 9109bdf into nabijaczleweli:master Aug 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Housekeeping - nothing to see here, move along #90

Housekeeping - nothing to see here, move along #90

mati865 commented Aug 7, 2018

nabijaczleweli left a comment •

edited

Loading

nabijaczleweli Aug 12, 2018

nabijaczleweli Aug 12, 2018

mati865 Aug 12, 2018

nabijaczleweli Aug 12, 2018

mati865 Aug 12, 2018

nabijaczleweli Aug 12, 2018

mati865 Aug 12, 2018

mati865 commented Aug 12, 2018

nabijaczleweli commented Aug 12, 2018

mati865 commented Aug 13, 2018

mati865 commented Aug 13, 2018

nabijaczleweli commented Aug 14, 2018 •

edited

Loading

Housekeeping - nothing to see here, move along #90

Housekeeping - nothing to see here, move along #90

Conversation

mati865 commented Aug 7, 2018

nabijaczleweli left a comment • edited Loading

Choose a reason for hiding this comment

nabijaczleweli Aug 12, 2018

Choose a reason for hiding this comment

nabijaczleweli Aug 12, 2018

Choose a reason for hiding this comment

mati865 Aug 12, 2018

Choose a reason for hiding this comment

nabijaczleweli Aug 12, 2018

Choose a reason for hiding this comment

mati865 Aug 12, 2018

Choose a reason for hiding this comment

nabijaczleweli Aug 12, 2018

Choose a reason for hiding this comment

mati865 Aug 12, 2018

Choose a reason for hiding this comment

mati865 commented Aug 12, 2018

nabijaczleweli commented Aug 12, 2018

mati865 commented Aug 13, 2018

mati865 commented Aug 13, 2018

nabijaczleweli commented Aug 14, 2018 • edited Loading

nabijaczleweli left a comment •

edited

Loading

nabijaczleweli commented Aug 14, 2018 •

edited

Loading