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

[WIP] Add support for running wasm-snip #163

Closed
wants to merge 8 commits into from
Closed

Conversation

csmoe
Copy link
Member

@csmoe csmoe commented Jun 11, 2018

Closes #158

Make sure these boxes are checked! 📦✅

  • You have the latest version of rustfmt installed and have your
    cloned directory set to nightly
$ rustup override set nightly
$ rustup component add rustfmt-preview --toolchain nightly
  • You ran rustfmt on the code base before submitting
  • You reference which issue is being closed in the PR text

✨✨ 😄 Thanks so much for contributing to wasm-pack! 😄 ✨✨

@ashleygwilliams
Copy link
Member

hey @csmoe ! thanks so much for this- it's looking really good. so we have a release coming up on wednesday and i think, unfortunately we're not gonna be able to sneak this in because there's some outstanding questions re #160 about configuration. i'm gonna make this as blocked for the moment, but the second after we get the most recent release out, i'll turn my attention back to this and see if we can't land it sooner than later and add the configuration option later so that we don't have to run you through too many rebases 😅 thanks again for submitting and sorry for having to ask you to wait, but hopefully it shouldn't be too long ✨ 💁

Copy link
Contributor

@mgattozzi mgattozzi left a comment

Choose a reason for hiding this comment

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

Real solid PR! Like Ashley said we'll need to hold off on this but this is great :D

src/error.rs Outdated Show resolved Hide resolved
src/error.rs Show resolved Hide resolved
src/command/snip.rs Outdated Show resolved Hide resolved
src/command/snip.rs Outdated Show resolved Hide resolved
@ashleygwilliams ashleygwilliams modified the milestones: 0.5.0, 0.6.0 Jul 26, 2018
@csmoe csmoe mentioned this pull request Aug 2, 2018
@csmoe csmoe changed the title cmd(snip): Add support for running wasm-snip [WIP] Add support for running wasm-snip Aug 2, 2018
@csmoe csmoe force-pushed the snip branch 2 times, most recently from 2e5a2c0 to a57d0a0 Compare August 2, 2018 10:31
@ashleygwilliams ashleygwilliams added the work in progress do not merge! label Sep 13, 2018
@nicksrandall
Copy link

@csmoe are you still planning to work on this? If you've run out of bandwidth, I'd be happy to help. I'd like to get this and wasm-opt integration in.

@csmoe
Copy link
Member Author

csmoe commented Jan 14, 2019

@nicksrandall really thanks. just do whatever you wanna.

@ashleygwilliams
Copy link
Member

this PR is quite outdated- i'm sorry about this :( would you mind if we closed? we still want this feature but i imagine it will be easier to start a new PR to accomplish this. let me know what you think!

@csmoe
Copy link
Member Author

csmoe commented Feb 22, 2019

yep It's better to restart

@csmoe csmoe closed this Feb 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for running wasm-snip
5 participants