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

Remove futures-nightly feature #373

Merged
merged 1 commit into from Aug 31, 2018

Conversation

Projects
None yet
3 participants
@sdroege
Member

sdroege commented Aug 30, 2018

futures 0.2 is not maintained anymore and the nightly parts don't work
anymore with latest nightly toolchain


If I understand how to optionally depend (based on a feature flag) on futures 0.3, I'll re-add this in a different way.

Does someone know how that would look like in Cargo.toml?

Remove futures-nightly feature
futures 0.2 is not maintained anymore and the nightly parts don't work
anymore with latest nightly toolchain
@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Aug 30, 2018

Member

Instead of removing code, wouldn't it be better to comment it?

Member

GuillaumeGomez commented Aug 30, 2018

Instead of removing code, wouldn't it be better to comment it?

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 30, 2018

Member

Personally I prefer removing, it usually give better results with git revert.

Member

EPashkin commented Aug 30, 2018

Personally I prefer removing, it usually give better results with git revert.

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Aug 30, 2018

Member

@sdroege Any opinion?

Member

GuillaumeGomez commented Aug 30, 2018

@sdroege Any opinion?

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Aug 30, 2018

Member

I removed it because for the future 0.3 version I'll simply go with a main_context_futures_nightly.rs or similar. There are too many differences to make it work in a reasonable way inside the same file.

Member

sdroege commented Aug 30, 2018

I removed it because for the future 0.3 version I'll simply go with a main_context_futures_nightly.rs or similar. There are too many differences to make it work in a reasonable way inside the same file.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 30, 2018

Member

About loading different crates version by feature:
Checked on other crate that [target.'cfg(feature="futures-nightly")'.dependencies] not works
and [target.'cfg(not(feature="futures-nightly"))'.dependencies] always included same as [dependencies]
😭

Member

EPashkin commented Aug 30, 2018

About loading different crates version by feature:
Checked on other crate that [target.'cfg(feature="futures-nightly")'.dependencies] not works
and [target.'cfg(not(feature="futures-nightly"))'.dependencies] always included same as [dependencies]
😭

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Aug 30, 2018

Member

Ok, let's go with removal then.

Member

GuillaumeGomez commented Aug 30, 2018

Ok, let's go with removal then.

@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 30, 2018

Member

Then it time to merge gtk-rs/examples#185 and restart CI here

Member

EPashkin commented Aug 30, 2018

Then it time to merge gtk-rs/examples#185 and restart CI here

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Aug 30, 2018

Member

Restarted travis.

Member

GuillaumeGomez commented Aug 30, 2018

Restarted travis.

@sdroege

This comment has been minimized.

Show comment
Hide comment
@sdroege

sdroege Aug 31, 2018

Member

See rust-lang/cargo#5954 for the missing cargo feature

Can we get this merged now, or ...? :)

Member

sdroege commented Aug 31, 2018

See rust-lang/cargo#5954 for the missing cargo feature

Can we get this merged now, or ...? :)

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Aug 31, 2018

Member

Sure, thanks!

Member

GuillaumeGomez commented Aug 31, 2018

Sure, thanks!

@GuillaumeGomez GuillaumeGomez merged commit 8321455 into gtk-rs:master Aug 31, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@EPashkin

This comment has been minimized.

Show comment
Hide comment
@EPashkin

EPashkin Aug 31, 2018

Member

@sdroege rust-lang/cargo#1197 active since 2015 😭
Also somewhere I see that it better not implemented since features can be set externally (by parents) and can produce conflicting set.

Member

EPashkin commented Aug 31, 2018

@sdroege rust-lang/cargo#1197 active since 2015 😭
Also somewhere I see that it better not implemented since features can be set externally (by parents) and can produce conflicting set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment