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

Clean up sc-network#9761

Merged
bkchr merged 7 commits intoparitytech:masterfrom
liuchengxu:clean-network
Sep 13, 2021
Merged

Clean up sc-network#9761
bkchr merged 7 commits intoparitytech:masterfrom
liuchengxu:clean-network

Conversation

@liuchengxu
Copy link
Copy Markdown
Contributor

This PR is mainly about cleaning up sc-network. No logical changes, all trivial improvements:

  • Avoid using clone() for the Copy type like PeerId.
  • Use find_map for filter_map and next.
  • Use Self.
  • Use matches! if suitable.
  • Make the log format look a bit more consistent at least per file.

- Avoid using clone() for the Copy type `PeerId`.
- Use `find_map` for `filter_map` and `next`.
- Use `Self`.
@bkchr bkchr requested a review from kpp September 13, 2021 11:55
@bkchr bkchr added A0-please_review Pull request needs code review. 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 labels Sep 13, 2021
.map(|_| default_notif_handshake_message.clone()),
)
.collect(),
vec![default_notif_handshake_message; params.network_config.extra_sets.len()],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, no. elems[0] should be an empty Vec.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try this:

fn main() {
    core::iter::once(Vec::<()>::new()).inspect(|x| {dbg!(&x);}).collect::<Vec<_>>();
}

Copy link
Copy Markdown
Contributor Author

@liuchengxu liuchengxu Sep 13, 2021

Choose a reason for hiding this comment

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

Reverted. Interesting, didn't realize the code is not equivalent as before, [[]] is not [], https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=ca2a7d77fe4d52b1e0ae709f20cdee83 . Do you mind sharing more about why elems[0] has to be an empty Vec?

Copy link
Copy Markdown
Contributor

@kpp kpp left a comment

Choose a reason for hiding this comment

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

There is an issue with vec![default_notif_handshake_message]. The other clean-ups are OK.

@bkchr bkchr merged commit 1fca377 into paritytech:master Sep 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. 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

Development

Successfully merging this pull request may close these issues.

3 participants