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

if target/wasm-bindgen already exists don't attempt to re-add/install it #51

Closed
ashleygwilliams opened this issue Mar 13, 2018 · 8 comments
Labels
PR attached there's a PR open for this issue to-do stuff that needs to happen, so plz do it k thx
Milestone

Comments

@ashleygwilliams
Copy link
Member

No description provided.

@ashleygwilliams ashleygwilliams added the to-do stuff that needs to happen, so plz do it k thx label Mar 13, 2018
@ashleygwilliams ashleygwilliams added this to the 0.1.0 milestone Mar 13, 2018
@data-pup
Copy link
Member

data-pup commented Apr 6, 2018

I would love to help out with this one if it wouldn't be too difficult for a first contribution! Would you have any advice for identifying whether or not target/wasm-bindgen exists already?

I'm guessing that this would mean adding a check to the cargo_install_wasm_bindgen function, but I'm not quite sure what the correct method is to identify whether wasm-bindgen has already been added.

@ashleygwilliams
Copy link
Member Author

hey @data-pup ! so, i wasn't actually entirely sure- this stack overflow answer seems mostly reasonable: https://stackoverflow.com/questions/35045996/check-if-a-command-is-in-path-executable-as-process. lemme know if you need help digesting it!

@data-pup
Copy link
Member

data-pup commented Apr 6, 2018

Hi! Thank you for the link, that definitely helped. I started working on this today, here is the branch with some first steps. I think I will need to add some extra logic for this to work on Windows, which uses ; to separate items in $PATH iirc.

Should the progress bar still show up if wasm-bindgen is already installed? Right now it just exits the cargo_install_wasm_bindgen function if wasm-bindgen is found in the path.

data-pup@7b45520

@ashleygwilliams
Copy link
Member Author

hey @data-pup ! sorry for the delay, i was out sick earlier this week- do you think you could open up a PR? it's much easier to give feedback :D looking forward to reviewing it and thanks so much!

@ashleygwilliams ashleygwilliams added the PR attached there's a PR open for this issue label Apr 11, 2018
@data-pup
Copy link
Member

Hi @ashleygwilliams! Sure thing, I can open a PR today. Thanks for helping me out with contributing, I really appreciate it and am looking forward to helping out more!

@ashleygwilliams
Copy link
Member Author

@data-pup can you run rustc --version and cargo fmt --version and let me know what the output is?

@data-pup
Copy link
Member

data-pup commented Apr 13, 2018

@ashleygwilliams Here are the versions of rustc and rust-fmt. These might be slightly out of date, I can update these and see if that fixes the issue.

✨  rustc --version
rustc 1.26.0-nightly (a04b88d19 2018-03-19)

✨  cargo fmt --version
0.3.8-nightly (346238f 2018-02-04)

Update: I updated my nightly toolchain, and formatted using these versions:

✨  rustc --version 
rustc 1.27.0-nightly (ad610bed8 2018-04-11)
✨  cargo fmt --version
rustfmt 0.4.1-nightly (e784712 2018-04-09)

This did not seem to fix the problem. The Travis build failed again :/

@fitzgen
Copy link
Member

fitzgen commented Aug 29, 2018

Fixed in #192

@fitzgen fitzgen closed this as completed Aug 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR attached there's a PR open for this issue to-do stuff that needs to happen, so plz do it k thx
Projects
None yet
Development

No branches or pull requests

3 participants