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

Add a --debug option to suppress cargo build --release. #127

Merged
merged 3 commits into from
Jun 12, 2018

Conversation

itsybitesyspider
Copy link
Contributor

First pass at #126 .

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! πŸ˜„ ✨✨

@itsybitesyspider itsybitesyspider changed the title Add a --debug option to suppress wasm-bindgen --release. Add a --debug option to suppress cargo --release. May 25, 2018
@itsybitesyspider itsybitesyspider changed the title Add a --debug option to suppress cargo --release. Add a --debug option to suppress cargo build --release. May 25, 2018
@itsybitesyspider
Copy link
Contributor Author

itsybitesyspider commented May 25, 2018

I don't know much about rustfmt but I assume the cargo fmt error is related to this recent work:

rust-lang/rustfmt@95d6b64

@mgattozzi
Copy link
Contributor

Yeah I think this is why. I've opened up an issue and will get a fix in for it.

@mgattozzi
Copy link
Contributor

If you can rebase this that would be great. We fixed the CI issue. I do think this needs to be discussed in #126 before we consider merging this though.

@itsybitesyspider
Copy link
Contributor Author

If you can rebase this that would be great.

βœ”οΈ

I do think this needs to be discussed in #126 before we consider merging this though.

Yep. I'm aware I kindof dropped this on you.

@ashleygwilliams ashleygwilliams added needs rebase needs docs please add docs to this PR labels May 29, 2018
@ashleygwilliams ashleygwilliams added this to the 0.4.0 milestone May 31, 2018
@ashleygwilliams
Copy link
Member

ashleygwilliams commented Jun 5, 2018

@clanehin hey! i'd love to merge this- it's gonna need a rebase and some documentation- are you up for it? let me know! (and if you need any help/guidance, please ask away!)

@itsybitesyspider
Copy link
Contributor Author

This should be rebased and doc'ed. I haven't done a lot of collaborative github in a while so if I've missed anything at this point then I'm oblivious to it.

@itsybitesyspider
Copy link
Contributor Author

As regards #153, I'll have to follow up maybe next week. Maybe sensible to let the upstream discussion solidify anyway?

@ashleygwilliams
Copy link
Member

thanks so much, checking this out locally and then i'll merge! πŸ‘Ύ 🐬 ✨

Copy link
Member

@ashleygwilliams ashleygwilliams left a comment

Choose a reason for hiding this comment

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

πŸš€ πŸ‘

@ashleygwilliams ashleygwilliams removed the needs docs please add docs to this PR label Jun 11, 2018
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.

LGTM as well :) Great job!

@ashleygwilliams ashleygwilliams merged commit d9fdd63 into rustwasm:master Jun 12, 2018
@itsybitesyspider
Copy link
Contributor Author

Yay, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants