Fix clippy warnings#1890
Conversation
|
I have a large branch that will conflict, I think it will be easier for you to rebase and eject a couple of your clippy fixes than the other way around - please hold off on merging a couple days. |
|
☔ The latest upstream changes (presumably #1876) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@lzutao Could you please rebase, and resubmit? |
|
Sorry for the delay! I pushed new changes. |
kinnison
left a comment
kinnison
left a comment
There was a problem hiding this comment.
Mostly looks sane. I only commented on the first Typename -> Self conversion, but I'd like to know which lint is making that desirable because to me it's less readable in some cases, though better in others.
A suggestion is present for some of your refactoring, otherwise looks good.
| let dist_root = dist_root_server.clone() + "/dist"; | ||
|
|
||
| let cfg = Cfg { | ||
| let cfg = Self { |
There was a problem hiding this comment.
Is this a new lint I was previously unaware of?
There was a problem hiding this comment.
Aah I see. This one is a pedantic lint and I'm not sure I completely care for it. I won't say "no" this time, but it feels a bit overly pedantic for my liking. Can make it harder to tell what something is if it's overused IMO.
204ebbc to
408f85b
Compare
|
Honestly, I don't know how |
|
The changes look good, but the build failed. And differently this time -- the |
|
The |
|
Oops! You forgot to squash this PR before merging. |
I didn't really see a need to squash it. Each commit was clear what it was doing. |
4 participants
Note: Please squash these commits before merge.