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.

Sign up for GitHub

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

Jump to bottom

Disable default features of stable_deref_trait #306

Merged
merged 1 commit into from
Jun 11, 2018
Merged

Disable default features of stable_deref_trait #306

merged 1 commit into from
Jun 11, 2018

Conversation

est31
Copy link
Contributor

@est31 est31 commented Jun 7, 2018

std is listed in the default features of stable_deref_trait

This was referenced Jun 7, 2018
Closed
Closed
@coveralls
Copy link

coveralls commented Jun 7, 2018 •
edited
Loading

Coverage Status

Coverage remained the same at 92.703% when pulling 6e12db3 on est31:master into 05e7be2 on gimli-rs:master.

fitzgen
fitzgen approved these changes Jun 7, 2018
View reviewed changes
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Regarding this comment, it isn't clear to me if this is ready to merge now or not.

@est31
Copy link
Contributor Author

est31 commented Jun 7, 2018

Given that @philipc said that we'd need that PR I've thought this PR wouldn't run successfully on CI but it did, so I'd say this PR is ready to merge. We certainly have to turn off the std feature of stable_deref_trait if our std feature is turned off.

Given that (against my expectations) CI ran successfully for my PR, I'd say that only #![no_std] users of gimli like addr2line need the PR that @philipc linked to. If that PR is merged and a new version of stable_deref_trait is released e.g. 1.0.1 it seems to me that we don't really need to change anything on the gimli side beyond this current PR.

@fitzgen
Copy link
Member

fitzgen commented Jun 7, 2018

Probably worth waiting for that PR to merge and publish first, so we can require that newly published point release to guarantee that no_std builds get the correct stable_deref_trait then.

@est31
Copy link
Contributor Author

est31 commented Jun 7, 2018

@fitzgen sure, let's wait

@est31
Disable default features of stable_deref_trait
6e12db3 
std is listed in the default features of stable_deref_trait

Also, update the requirement to 1.1.0
as it implements some needed traits.
@est31
Copy link
Contributor Author

est31 commented Jun 10, 2018

The PR Storyyeller/stable_deref_trait#2 merged and the upstream author made a new release. I've updated the PR to reflect that. re-r? @fitzgen

fitzgen
fitzgen approved these changes Jun 11, 2018
View reviewed changes
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@fitzgen fitzgen merged commit 2544b79 into gimli-rs:master Jun 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fitzgen fitzgen fitzgen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@est31 @coveralls @fitzgen