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

Fix most of clippy warnings #1754

Merged
merged 1 commit into from
Apr 14, 2019
Merged

Fix most of clippy warnings #1754

merged 1 commit into from
Apr 14, 2019

Conversation

tesuji
Copy link
Contributor

@tesuji tesuji commented Apr 11, 2019

Note: Feel free to squash this PR.

There are 3 remained warnings:

  • large_enum_variant
  • module_inception
  • should_implement_trait
Remaining clippy warnings
warning: large size difference between variants
   --> src/errors.rs:17:1
    |
17  | / error_chain! {
18  | |     links {
19  | |         Download(download::Error, download::ErrorKind);
20  | |     }
...   |
322 | |     }
323 | | }
    | |_^
    |
    = note: #[warn(clippy::large_enum_variant)] on by default
help: consider boxing the large fields to reduce the total size of the enum
   --> src/errors.rs:17:1
    |
17  | / error_chain! {
18  | |     links {
19  | |         Download(download::Error, download::ErrorKind);
20  | |     }
...   |
322 | |     }
323 | | }
    | |_^
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#large_enum_variant
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

warning: module has the same name as its containing module
 --> src/dist/mod.rs:9:1
  |
9 | pub mod dist;
  | ^^^^^^^^^^^^^
  |
  = note: #[warn(clippy::module_inception)] on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#module_inception

warning: module has the same name as its containing module
 --> src/utils/mod.rs:6:1
  |
6 | pub mod utils;
  | ^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#module_inception


warning: defining a method called `from_str` on this type; consider implementing the `std::str::FromStr` trait or choosing a less ambiguous name
   --> src/dist/dist.rs:113:5
    |
113 | /     pub fn from_str(name: &str) -> Self {
114 | |         TargetTriple(name.to_string())
115 | |     }
    | |_____^
    |
    = note: #[warn(clippy::should_implement_trait)] on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#should_implement_trait

warning: defining a method called `from_str` on this type; consider implementing the `std::str::FromStr` trait or choosing a less ambiguous name
   --> src/dist/dist.rs:213:5
    |
213 | /     pub fn from_str(name: &str) -> Option<Self> {
214 | |         if name.is_empty() {
215 | |             return Some(PartialTargetTriple {
216 | |                 arch: None,
...   |
248 | |         })
249 | |     }
    | |_____^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#should_implement_trait

warning: defining a method called `from_str` on this type; consider implementing the `std::str::FromStr` trait or choosing a less ambiguous name
   --> src/dist/dist.rs:253:5
    |
253 | /     pub fn from_str(name: &str) -> Result<Self> {
254 | |         let channels = [
255 | |             "nightly",
256 | |             "beta",
...   |
290 | |         }
291 | |     }
    | |_____^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#should_implement_trait

warning: defining a method called `from_str` on this type; consider implementing the `std::str::FromStr` trait or choosing a less ambiguous name
   --> src/dist/dist.rs:343:5
    |
343 | /     pub fn from_str(name: &str) -> Result<Self> {
344 | |         let channels = [
345 | |             "nightly",
346 | |             "beta",
...   |
374 | |             .ok_or(ErrorKind::InvalidToolchainName(name.to_string()).into())
375 | |     }
    | |_____^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#should_implement_trait
</details>

@tesuji tesuji force-pushed the clippy branch 3 times, most recently from 90a6287 to f1ae991 Compare April 11, 2019 16:49
@kinnison
Copy link
Contributor

Wow, this is a big PR. It will need to take quite a bit of time to be able to review it all. But thank you for doing this work.

If anyone else wants to review this, please please tell me, otherwise I'll try and set aside an hour or so soon.

@tesuji
Copy link
Contributor Author

tesuji commented Apr 13, 2019

Rebased on master.

@bors
Copy link
Contributor

bors commented Apr 14, 2019

☔ The latest upstream changes (presumably #1744) made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Member

@nrc nrc left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! I had one comment, but you can ignore it. Needs a rebase

src/cli/rustup_mode.rs Outdated Show resolved Hide resolved
@tesuji
Copy link
Contributor Author

tesuji commented Apr 14, 2019

Rebased on master.

@kinnison
Copy link
Contributor

If you could fold the formatting updates into whichever commit they belong then I'm OK to merge this

@kinnison
Copy link
Contributor

Did you mean to squash the entire branch into one commit? If so then that's fine, but if you meant to keep the different fixes independent then you might want to sort that out :D

@tesuji
Copy link
Contributor Author

tesuji commented Apr 14, 2019

I squashed this to one commit because all those small commits only serve
one purpose: fix clippy warnings.
It's ready to merge now.

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.

4 participants