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

Disable default features on Rocket #69

Merged
merged 1 commit into from
Aug 28, 2019
Merged

Disable default features on Rocket #69

merged 1 commit into from
Aug 28, 2019

Conversation

jhpratt
Copy link
Contributor

@jhpratt jhpratt commented Aug 28, 2019

This change should allow using rocket_cors with Rocket's async branch, which has (indirectly) updated its dependency on ring. Since rocket_cors doesn't use cookies anywhere, this change effectively does nothing but increase compatibility.

@lawliet89
Copy link
Owner

LGTM but I am not sure if this will cause issues when someone has

  • Rocket in their dependency tree with default features enabled
  • and passes a type constructed with their copy of Rocket into this library which only understands Rocket types with default features enabled.

@jhpratt
Copy link
Contributor Author

jhpratt commented Aug 28, 2019

I'm fairly certain cargo takes the greatest common denominator, so it would enable the feature if necessary, but leave if out otherwise.

@lawliet89
Copy link
Owner

The build will fail on an older nightly version. Will merge it once the tests pass on latest nightly.

I will go fix that... some day.

@jhpratt
Copy link
Contributor Author

jhpratt commented Aug 28, 2019

Ok. I tested it locally before creating this PR and everything passed. That's on a recent (~1 week old) nightly.

@lawliet89
Copy link
Owner

Failed due to new clippy lints. I will fix them some time later.

@lawliet89 lawliet89 merged commit c75dcb2 into lawliet89:master Aug 28, 2019
v1olen pushed a commit to v1olen/rocket_cors that referenced this pull request Mar 18, 2021
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