Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

a few lints more #12049

Closed
gilescope wants to merge 13 commits into from
Closed

a few lints more #12049

gilescope wants to merge 13 commits into from

Conversation

@gilescope
Copy link
Contributor

@gilescope gilescope commented Aug 16, 2022 •
edited
Loading

This PR follows up on #11154 and adds hopefully non-controversial lints to the list.

Have removed move from one location and removed an unsafe.

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Aug 16, 2022
gilescope
gilescope commented Aug 16, 2022
View reviewed changes
@gilescope gilescope added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Aug 16, 2022
@gilescope gilescope requested a review from bkchr August 16, 2022 15:59
@gilescope gilescope added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label Aug 17, 2022
gilescope
gilescope commented Aug 17, 2022
View reviewed changes
@paritytech-cicd-pr
Copy link

paritytech-cicd-pr commented Aug 17, 2022

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - test-linux-stable
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1753216

kianenigma
kianenigma reviewed Aug 18, 2022
View reviewed changes
kianenigma
kianenigma reviewed Aug 18, 2022
View reviewed changes
kianenigma
kianenigma reviewed Aug 18, 2022
View reviewed changes
kianenigma
kianenigma approved these changes Aug 18, 2022
View reviewed changes
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Everything in /frame LGTM.

muharem
muharem reviewed Aug 19, 2022
View reviewed changes
gilescope
gilescope commented Aug 25, 2022
View reviewed changes
@gilescope
Copy link
Contributor Author

gilescope commented Aug 25, 2022

Going to wait a while longer and do another merge update and see what the changes are. In that last batch it was mostly needless_borrow that was triggering. I think maybe that's a lint too far - it's nice to have less & for readability but probably it breaks the flow a bit too much and would trigger too frequently.

@stale
Copy link

stale bot commented Sep 24, 2022

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Sep 24, 2022
@gilescope
Copy link
Contributor Author

gilescope commented Sep 25, 2022 via email

acatangiu
acatangiu approved these changes Sep 26, 2022
View reviewed changes
Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

Code changes look good, extra lints look good as well

@stale stale bot removed the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Sep 26, 2022
@stale
Copy link

stale bot commented Oct 26, 2022

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Oct 26, 2022
@stale stale bot closed this Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@athei athei Awaiting requested review from athei athei is a code owner

@andresilva andresilva Awaiting requested review from andresilva

@bkchr bkchr Awaiting requested review from bkchr

3 more reviewers

@muharem muharem muharem left review comments

@kianenigma kianenigma kianenigma approved these changes

@acatangiu acatangiu acatangiu approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A0-please_review Pull request needs code review. A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@gilescope @paritytech-cicd-pr @kianenigma @muharem @acatangiu