-
Notifications
You must be signed in to change notification settings - Fork 182
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
Enforce cargo fmt #199
Enforce cargo fmt #199
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
Feel free to merge some things in ahead of this and I'll rebase.. it'll be easier that way. |
Travis seems grumpy. One thing:
Other things:
although those things might just be bugs that were fixed |
chalk-solve/src/infer.rs
Outdated
use chalk_ir::fold::Fold; | ||
use chalk_ir::*; | ||
use ena::unify as ena; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The leading ::
got removed.
829bf80
to
fe53397
Compare
Oops, my nightly was outdated. Trying again. I also enable Travis caching of cargo in this PR. I don't anticipate any problems with this but if you know of any I'll take it out. |
@@ -2,9 +2,13 @@ language: rust | |||
rust: | |||
- beta | |||
- nightly | |||
cache: cargo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will grow the cache every time a different nightly is used, because of rust-lang/cargo#6229
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we care ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is, I don't really know what the implications of that are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cache growth will cause CI time to go up, because the whole target dir has to been downloaded, unpacked, repacked and uploaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is less than ideal, but if the cache grows too big, we have a button on Travis to delete all repo caches and start fresh.
My expectation is this will save some time, both for installing rustfmt and downloading/building dependencies. But we can see the difference in build times once this is merged and decide to disable it again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems good to me! I'd like to land this, but I'm not sure whether to worry about the caching issue that @bjorn3 noted. =)
To be honest, I don't entirely know what cache: cargo
does.
I commented on the thread; I think we can leave it in for now. |
Closes #198.