-
Notifications
You must be signed in to change notification settings - Fork 95
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
fix: prevent compiling wasm only code on unsupported platforms #225
Conversation
Publish dry run is failing, maybe all features are turned on? Reqwest removed this function completely: seanmonstar/reqwest@0bcba46 |
reqwest-middleware/src/client.rs
Outdated
@@ -527,6 +527,7 @@ impl RequestBuilder { | |||
/// The [request mode][mdn] will be set to 'no-cors'. | |||
/// | |||
/// [mdn]: https://developer.mozilla.org/en-US/docs/Web/API/Request/mode | |||
#[cfg(target_family = "wasm")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reqwest uses this cfg attribute https://github.com/seanmonstar/reqwest/blob/master/src/lib.rs#L255C9-L255C39:
#[cfg(target_arch = "wasm32")]
I wonder if the issue is that these don't match
See also seanmonstar/reqwest#2596 Fixes TrueLayer#224 Signed-off-by: R. Tyler Croy <[email protected]>
2219899
to
a7597a7
Compare
@benbrandt should this PR be reviewed and obtained an approval from you for it to get merged? |
@douggynix I doubt it because I'm not a maintainer but I did anyway |
Can we include the maintainers in the comments to have this PR reviewed. I am having build errors which this PR has fixed. A review to update the library crate would be very useful for me and others experiencing build issues with the latest versions of reqwest. |
@eopb : would you have a look at this PR for approbation or change request please? |
Co-authored-by: Jonathan Donaldson <[email protected]>
@rtyler : you got pending requested changes to fix the CI issue. any update on this please? |
why this PR is still pending for so long knowing that there are build failures issues for the new versions of reqwest? |
Closing as this has now been fixed in reqwest-middleware 0.4.2 I didn't have permission to push changes to this branch, so I've fixed this in a separate PR: #230. |
See also seanmonstar/reqwest#2596
Fixes #224