Fix clippy warnings #1890
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. |
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.
| @@ -95,7 +95,7 @@ impl Cfg { | |||
| ); | |||
| 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.
I just enabled the lint in this PR.
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.
tesuji
force-pushed
the
fix-clippy
branch
2 times, most recently
from
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. |
Note: Please squash these commits before merge.