rspotify icon indicating copy to clipboard operation
rspotify copied to clipboard

Unbound Send marker if compile target is wasm32

Open tomocrafter opened this issue 3 years ago • 7 comments

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

tomocrafter avatar Dec 11 '21 12:12 tomocrafter

Oh interesting, so you don't need to toggle Send for these: https://github.com/ramsayleung/rspotify/blob/master/src/clients/base.rs#L262?

marioortizmanero avatar Dec 11 '21 12:12 marioortizmanero

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.

marioortizmanero avatar Dec 11 '21 12:12 marioortizmanero

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.

marioortizmanero avatar Dec 11 '21 12:12 marioortizmanero

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

ramsayleung avatar Dec 12 '21 01:12 ramsayleung

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.

shiguma127 avatar Dec 15 '21 14:12 shiguma127

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.

ramsayleung avatar Dec 16 '21 13:12 ramsayleung

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.

marioortizmanero avatar Dec 17 '21 15:12 marioortizmanero

Message to comment on stale PRs. If none provided, will not mark PRs stale

github-actions[bot] avatar Jul 02 '23 02:07 github-actions[bot]