Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify SyncingEngine::new() a bit #5396

Merged
merged 9 commits into from
Aug 30, 2024
Merged

Simplify SyncingEngine::new() a bit #5396

merged 9 commits into from
Aug 30, 2024

Conversation

liuchengxu
Copy link
Contributor

Tiny changes to simplify the code:

  • Remove an unnecessary collect.
  • Reduce the code duplication a little bit.

@liuchengxu
Simplify SyncingEngine::new() a bit
9df4c80 
Tiny changes to simplify the code:

- Remove an unnecessary `collect`.
- Reduce the code duplication a little bit.
@bkchr bkchr requested a review from a team August 21, 2024 21:44
@bkchr
Copy link
Member

bkchr commented Aug 21, 2024

@liuchengxu we need a prdoc. Can be a bump none as these are just cosmetics.

@bkchr bkchr added the T0-node This PR/Issue is related to the topic “node”. label Aug 21, 2024
lexnv
lexnv reviewed Aug 22, 2024
View reviewed changes
prdoc/pr_5396.prdoc Outdated Show resolved Hide resolved
lexnv
lexnv approved these changes Aug 22, 2024
View reviewed changes
Copy link
Contributor

@lexnv lexnv left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for contributing 🙏

@liuchengxu @lexnv
Update prdoc/pr_5396.prdoc
b1039f7 
Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
@bkchr bkchr added this pull request to the merge queue Aug 28, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 28, 2024
@liuchengxu
Copy link
Contributor Author

The link-checker failed persistently somehow.

@dmitry-markin dmitry-markin added this pull request to the merge queue Aug 30, 2024
@dmitry-markin dmitry-markin removed this pull request from the merge queue due to a manual request Aug 30, 2024
@dmitry-markin
Copy link
Contributor

The link-checker failed persistently somehow.

I don't think it's needed to merge the PR.

Can you revert the unrelated changes with replacing return -> return; in many places please?

@dmitry-markin
Copy link
Contributor

bot fmt

@command-bot
Copy link

command-bot bot commented Aug 30, 2024 •
edited
Loading

@dmitry-markin https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7193320 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 35-4eab844a-b24f-4ad2-9749-5fddc3cc3306 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Aug 30, 2024

@dmitry-markin Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7193320 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7193320/artifacts/download.

@dmitry-markin
Copy link
Contributor

Can you revert the unrelated changes with replacing return -> return; in many places please?

Never mind, it looks like we have a code style config updated.

@dmitry-markin dmitry-markin added this pull request to the merge queue Aug 30, 2024
Merged via the queue into paritytech:master with commit 3d4a335 Aug 30, 2024
184 of 186 checks passed
mordamax pushed a commit that referenced this pull request Aug 30, 2024
@liuchengxu @lexnv @mordamax
Simplify SyncingEngine::new() a bit (#5396)
31f2375 
Tiny changes to simplify the code:

- Remove an unnecessary `collect`.
- Reduce the code duplication a little bit.

---------

Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
@ggwpez ggwpez mentioned this pull request Sep 9, 2024
Closed
@github-actions github-actions bot mentioned this pull request Oct 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nazar-pc nazar-pc nazar-pc left review comments

@lexnv lexnv lexnv approved these changes

@alexggh alexggh alexggh approved these changes

@dmitry-markin dmitry-markin dmitry-markin approved these changes

@bkchr bkchr bkchr approved these changes

Assignees
No one assigned
Labels
T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@liuchengxu @bkchr @dmitry-markin @nazar-pc @alexggh @lexnv