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

Clean up dependencies #164

Merged
merged 5 commits into from Apr 22, 2020
Merged

Clean up dependencies #164

merged 5 commits into from Apr 22, 2020

Conversation

goto-bus-stop
Copy link
Member

  • Remove dependencies that had been moved into http-client.
  • Use the http-client/*_client features for curl/wasm/h1.
  • Remove mentions of hyper entirely as it is not currently implemented.
  • Test both native-client and h1-client on CI.

Remove dependencies that had been moved into http-client.

Use the http-client features for curl/wasm/h1.

Remove mentions of hyper entirely as it is not currently implemented.
middleware-logger = []
encoding = ["encoding_rs"]
# requires web-sys for TextDecoder on wasm
encoding = ["encoding_rs", "web-sys"]
Copy link
Member

Choose a reason for hiding this comment

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

Won't this also pull in web-sys on non-wasm targets? I believe if wasm-client has been enabled then web-sys is already available.

Copy link
Member Author

Choose a reason for hiding this comment

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

web-sys is still listed as a dependency for wasm targets only—appears to work as expected this way. actually web-sys is no longer directly used by surf for the wasm-client feature, so we can remove it from there

Comment on lines -66 to -78
"AbortSignal",
"Headers",
"ObserverCallback",
"ReferrerPolicy",
"Request",
"RequestCache",
"RequestCredentials",
"RequestInit",
"RequestMode",
"RequestRedirect",
"Response",
"TextDecoder",
"Window",
Copy link
Member

Choose a reason for hiding this comment

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

Oh wow; are these features no longer needed? That's huge!

Copy link
Member Author

Choose a reason for hiding this comment

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

They're all specified in http-client :D

Copy link
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

LGTM overall!

@goto-bus-stop goto-bus-stop merged commit 034440e into master Apr 22, 2020
@goto-bus-stop goto-bus-stop deleted the remove-unused-deps branch April 22, 2020 10:15
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