-
Notifications
You must be signed in to change notification settings - Fork 358
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fix clippy warning #1638
Fix clippy warning #1638
Conversation
674f57f
to
a9346c0
Compare
#1639 filed. Before it is fixed, we just need to be careful :) |
.github/workflows/main.yml
Outdated
- './tests/rust-integration-tests/integration_test' | ||
tests/rust-integration-tests/test_framework: | ||
- './tests/rust-integration-tests/test_framework' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, should we add /**
to these as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was trying this PR with #1643 to see if the rules gets triggered, since this PR contains changes to the integration test file. Since #1643 included the change first, I am letting that PR to merge this change. Then I can rebase this PR. Sorry for the confusion. This PR only fixes the clippy issue and all CI related change should be made in #1643
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, great 👍
Also, nice to see you back 😄
Signed-off-by: Eric Fang <yihuaf@unkies.org>
cf32ef6
to
27fde0a
Compare
@YJDoc2 Since we got you here, would you mind approve this? CI is currently blocked because the clippy warning this PR fixes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 👍
Thanks a lot!
Hey @yihuaf , We should go ahead and merge this. Is there anything that this depends on (other pr etc.)? |
No, this can now go in. I will merge. Thank you :) |
Not sure why these slipped through the crack. I will also file an issue to track why CI did not catch these.