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.

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

Merged
merged 5 commits into from
Oct 5, 2018
Merged

Improve Travis-CI config, fix warning and update to newest nightly #34

merged 5 commits into from
Oct 5, 2018

Conversation

LukasKalbertodt
Copy link
Member

@LukasKalbertodt LukasKalbertodt commented Sep 25, 2018

@LukasKalbertodt LukasKalbertodt changed the title WIP: Improve Travis-CI config Improve Travis-CI config Sep 25, 2018
Copy link
Member

@KodrAus KodrAus left a 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

@LukasKalbertodt
Copy link
Member Author

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.
@LukasKalbertodt LukasKalbertodt changed the title Improve Travis-CI config Improve Travis-CI config, fix warning and update to newest nightly Oct 4, 2018
@LukasKalbertodt
Copy link
Member Author

LukasKalbertodt commented Oct 4, 2018

@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.

Copy link
Member

@KodrAus KodrAus left a 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?

@LukasKalbertodt LukasKalbertodt merged commit 16e0c28 into auto-impl-rs:master Oct 5, 2018
@LukasKalbertodt LukasKalbertodt deleted the improve-travis branch October 5, 2018 10:17
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.

2 participants