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

Update dependencies #227

Merged
merged 2 commits into from
Aug 30, 2019
Merged

Update dependencies #227

merged 2 commits into from
Aug 30, 2019

Conversation

CreepySkeleton
Copy link
Collaborator

syn, proc_macro2 and quote were updated recently. Since you're going to publish a breaking version anyway we could ship this update along. Anyway, we will have to do it sooner or later and now it's a perfect opportunity.

@TeXitoi
Copy link
Owner

TeXitoi commented Aug 15, 2019

Any idea why the span of Option<Option<T>> is different on nightly? (that's why travis is not happy)

@TeXitoi
Copy link
Owner

TeXitoi commented Aug 15, 2019

Your git config seems also unsync with your git account.

@CreepySkeleton
Copy link
Collaborator Author

CreepySkeleton commented Aug 16, 2019

Your git config seems also unsync with your git account.

Oops, thanks for pointing out :)

Any idea why the span of Option<Option<T>> is different on nightly? (that's why travis is not happy)

It's about improvements in proc-macro2::Span: earlier it haven't had all the info necessary to highlight all the parts of a complex type at once on stable but it seems they now have some magic on nightly that allows it.

We have multiple ways to address this issue

  • We could test cargo build but not cargo test on nightly - if it works on stable it probably should work on nightly. We may roll things back once they're settled out.
  • We could use
    allow_failures:
        - rust: nightly
    
    in travis.yml and roll back when it's fixed.
  • We could make different test cases for stable and nightly - waste of efforts if you ask me, but I still would do it if you want.

I leave it at your discretion, tell me what you think and I'll update the PR

@CreepySkeleton
Copy link
Collaborator Author

CreepySkeleton commented Aug 20, 2019

@TeXitoi So what do we do about it? I suggest allow_failure or "separate test suits for stable and nightly" solutions: both are easy to implement and unroll when needed

@TeXitoi
Copy link
Owner

TeXitoi commented Aug 20, 2019

Sorry. Can we do failure only on stable? That would be perfect. Else, let's do tests only on stable, as it will pose problems later when the release train change if we just allow failure on nightly.

@TeXitoi
Copy link
Owner

TeXitoi commented Aug 20, 2019

"failure only on stable" I mean ui tests only on stable.

@CreepySkeleton
Copy link
Collaborator Author

I'm not sure I understand you correctly, are you're saying you want not to run ui tests on nightly but run all the other tests? If yes, that's exactly what's in this PR now.

@CreepySkeleton
Copy link
Collaborator Author

@TeXitoi Unfortunately, this is not all sunshine and bubbles. clap implicitly depends on failure and failure_derive crates, and they in turn depend on old (pre 1.*) versions of quote/proc_macro2/syn crates (they are in the process of updating though). So now we have to double-build syn/proc_macro2/quote crates. Let's wait for a few days before merging this (I'll poke these guys tomorrow and I'll offer my help to speed up things)

@CreepySkeleton
Copy link
Collaborator Author

By the way, it seems like nightly feature is obsolete:

https://github.com/alexcrichton/proc-macro2/blob/75972f61403c6cc453608f3358cbfdef6edea4b5/Cargo.toml#L39

@TeXitoi
Copy link
Owner

TeXitoi commented Aug 23, 2019

We can remove the nightly feature.

Don't run ui tests on nightly

Separate folders

wip
@CreepySkeleton
Copy link
Collaborator Author

@TeXitoi This seems complete. You can merge it once people manage to update their deps

@TeXitoi
Copy link
Owner

TeXitoi commented Aug 30, 2019

As removing the nightly feature is a breaking change, we must merge this before 0.3. I don't really care if for now the user need to compile 2 times syn and quote as using the 1 version is the thing to do and the update of the deps will be transparent.

Can you update the changelog about the removal of the nightly feature, and I'll merge this.

@TeXitoi
Copy link
Owner

TeXitoi commented Aug 30, 2019

I'm OK to merge this now if you update the changelog in the other PR.

@TeXitoi TeXitoi merged commit b57dde1 into TeXitoi:master Aug 30, 2019
@TeXitoi
Copy link
Owner

TeXitoi commented Aug 30, 2019

Thanks!

@kpcyrd kpcyrd mentioned this pull request Aug 30, 2019
@CreepySkeleton CreepySkeleton deleted the update_deps branch September 2, 2019 12:25
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.

None yet

2 participants