-
Notifications
You must be signed in to change notification settings - Fork 14
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
Improve Travis-CI config, fix warning and update to newest nightly #34
Improve Travis-CI config, fix warning and update to newest nightly #34
Conversation
LukasKalbertodt
commented
Sep 25, 2018
•
edited
Loading
edited
- Improves Travis configuration (see commit message for more information)
- Fixes a warning introduced by Addressing T19, and adding explicit generic type parameters to delegated methods #35 (I did miss that warning when reviewing the PR :<)
- Updates to newest nightly: this just adds a new feature required for the "nightly" mode of this crate
- Fix usage of buildplan with nightly feature
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.
Thanks @LukasKalbertodt! This looks good to me
The Cargo fix will land in nightly once this PR is merged. |
Several improvements: - Split "without nightly feature" and "with nightly feature" into two jobs - Add a job that builds on beta - Fix `RUST_FLAGS` => `RUSTFLAGS` typo - Remove cargo cache. Caching is not always good. Download the cache takes time and since we compile with different feature sets, the `target` folder cannot really be reused anyway. We should keep an eye on this, though. - Early exit in scripts (fail when first command fails)
PR #35 introduced a warning in one of the examples. We were passing all generic parameters to the inner function, including lifetimes. But passing late-bound lifetimes won't be allowed in the future. This commit simply ignores lifetime parameters. AFAICT this should never be a problem. Passing lifetimes explicitly hardly ever makes sense. The only case I can think of where the changes of this commit could break anything is in this function: fn foo<'a>() -> &'a i32 { ... } But functions with an early-bound lifetime (return type) and no late-bound ones are extremely rare and special. So for now we can ignore them. In particular, determining which lifetime parameters are early- and which are late-bound is extremely difficult. Lastly, const parameters are not yet correctly passed to the inner function. This needs to be fixed in the future, when const parameters are a thing.
@KodrAus This PR is ready now. Finally a green CI build ^_^ The PR grew larger than expected. So I would appreciate it if you could take a quick look again. Several fixes here and there. |
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.
Looks good 👍 It might be worth adding a little inline comment that we're not passing lifetimes because some of then might be late-bound, and we'd rather let the compiler figure it out implicitly?