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

Refactor main.rs #78

Merged
merged 4 commits into from
Apr 13, 2018
Merged

Conversation

Andy-Bell
Copy link
Contributor

This pull request is designed to move all the functionality out of the main.rs binary and into the main library itself - for testability and to hopefully provide some help with https://github.com/ashleygwilliams/wasm-pack/issues/25 .

Currently throws warning due to line 11 in command.rs - unused import of quicli - import is used for the StructOpt derive macro, so is needed to be imported, but due to no structs/functions from quicli being usied it still throws this warning.

to lib.rs to hopefully allow for better use of
lazy_static as discussed in issue rustwasm#25
Functions to new module. Tidies up reused code into
private function.

Throws warning due to line 11 in command.rs - unused import
Import is used for the StructOpt derive macro, so is needed
but still throws this warning
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.

i really dig this. pending an appeasement of cargo fmt i'm all for merging this in.

@ashleygwilliams
Copy link
Member

curious about ur thoughts @mgattozzi

@mgattozzi
Copy link
Contributor

@ashleygwilliams I'm of the opinion that main.rs should really only be a file that sets up and drives the program and was going to suggest we do this at some point anyways. It makes things easier to reason about so consider this an LGTM

@ashleygwilliams ashleygwilliams merged commit 0f01272 into rustwasm:master Apr 13, 2018
@Andy-Bell Andy-Bell deleted the refactor-main-rs branch April 13, 2018 12:17
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