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: fix clippy warnings #478

Merged
merged 2 commits into from
Feb 22, 2019

Conversation

mstallmo
Copy link
Member

closes #477

This is a first pass PR at cleaning up the clippy warnings. Most of them were simple enough refactors that it only took minor modifications to the code to get rid of the warnings. There were a few warnings, marked by #![allow()], that would require more significant changes to the code structure so instead of going ahead and implementing those changes I marked them as clippy allow until we can decide if the changes are something we want or not.


Make sure these boxes are checked! 📦✅

  • You have the latest version of rustfmt installed
$ rustup component add rustfmt-preview --toolchain nightly
  • You ran cargo fmt 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! 😄 ✨✨

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.

this is great! sorry for the delay in reviewing. we should def make a second pass and see if we can't eliminate some of those allows. thank you so much!

@mstallmo
Copy link
Member Author

Awesome! I should be able to fix up the conflicts this evening so everything will be ready to merge.

I'll also go ahead and file a new issue for working on removing the allows !

@ashleygwilliams
Copy link
Member

@mstallmo i might be trying to get this into 0.6.0- would you mind if i took over and fixed the conflicts? i am just feeling guilty and don't want to delay the release any further <3

@ashleygwilliams
Copy link
Member

actually, @mstallmo i'm gonna hold off and push this to 0.7.0- which should only be a month. go ahead and work on the conflict and we can get this merged in as one of the first PRs post release!

@ashleygwilliams ashleygwilliams modified the milestones: 0.6.0, 0.7.0 Jan 15, 2019
@mstallmo
Copy link
Member Author

@ashleygwilliams Sorry for the delayed response but that sounds great!

@mstallmo
Copy link
Member Author

Just pushed the update resolving the merge conflicts with master so this should be good to go as soon as the checks pass.

@ashleygwilliams
Copy link
Member

i need to do #509 before this because it's more urgent- which may introduce another conflict- but i'll handle that for you! thanks so much for your work here!

@ashleygwilliams
Copy link
Member

since we opted against a point release, we can get this into the next release (0.7.0) thanks!

@ashleygwilliams ashleygwilliams merged commit 53d0754 into rustwasm:master Feb 22, 2019
@mstallmo mstallmo deleted the clippy-refactor branch March 2, 2019 19:11
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.

Fix clippy warnings in the codebase
2 participants