rspotify
rspotify copied to clipboard
Unbound Send marker if compile target is wasm32
Description
Unbounds Send marker if compile target is wasm32 to fix #292
Motivation and Context
Please also include relevant motivation and context.
Dependencies
List any dependencies that are required for this change.
Type of change
Please delete options that are not relevant.
- [x] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [x] This change requires a documentation update
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
Is this change properly documented?
This PR doesn't updated document. and I'm not used to write document in English proper so if anyone can, please create PR.
Please make sure you've properly documented the changes you're making.
Don't forget to add an entry to the CHANGELOG if necessary (new features, breaking changes, relevant internal improvements). TBD
Oh interesting, so you don't need to toggle Send
for these: https://github.com/ramsayleung/rspotify/blob/master/src/clients/base.rs#L262?
Also, are you familiar with how GitHub Actions work? Could you perhaps add wasm32-unknown-unknown
to our CI tests? I can help you out otherwise, but another day.
And finally, it'd be nice if you could add some wasm32-unknown-unknown
specific tests if possible. You can write them similarly to however you're using rspotify
yourself for your project.
As Mario mentioning above, you could take the original cross-compile CI configuration(which has been removed) as an example, in case you have no idea how does our CI work
Created a test. This is my first time using HithubActions CI, so there are some things that are not good. If you have any other ideas or good ways to do it, please let me know. Thank you.
You could convert this PR to the stage of ready for review
, and then the GitHub Action CI will run. Later, You could check if it works.
Oh, nice. I was going to ask you to use matrix
in the CI as we did before, but it seems the tests are ran in a very different way than with wasm, so I think it's ok in this case to add a new flow from scratch. I've approved the CI run to see if it passes, but I still wouldn't merge this PR until we review it in detail.
Message to comment on stale PRs. If none provided, will not mark PRs stale