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

Conversation

@JesseAbram
Copy link
Contributor

@JesseAbram JesseAbram commented Feb 26, 2021

This PR implements the on_idle hook as discussed in issue #4064

@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Feb 26, 2021
@apopiak apopiak mentioned this pull request Feb 26, 2021
@athei athei linked an issue Feb 26, 2021 that may be closed by this pull request
@gui1117
Copy link
Contributor

gui1117 commented Mar 1, 2021

should we allow decl_module to implement on_idle? I think we can just make decl_module always define the default on_idle and not allow user to use its own implementation.

EDIT: but now that it is implemented, we can keep it.

@athei athei changed the title add in idle hook Add on_idle hook Mar 1, 2021
Co-authored-by: Guillaume Thiolliere <[email protected]>
Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

looks good, some doc nitpicks.

Also doc of expansion of pallet macro should be updated: https://substrate.dev/rustdocs/v3.0.0/frame_support/attr.pallet.html#macro-expansion-2

/// The block is being finalized. Implement to have something happen.
fn on_finalize(_n: BlockNumber) {}

/// The block is being finalized. Implement to have something happen using the remaining weight.
Copy link
Contributor

@gui1117 gui1117 Mar 9, 2021

Choose a reason for hiding this comment

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

I think you can copy/paste all the doc of OnIdle here and also link to it, something like:

"The function can be used to implement [OnIdle]."

Because pallet user probably want to read doc from this trait.

Co-authored-by: Alexander Popiak <[email protected]>
Copy link
Contributor

@apopiak apopiak left a comment

Choose a reason for hiding this comment

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

LGTM, some minor docs nitpicks

JesseAbram and others added 3 commits March 11, 2021 17:01
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. C1-low PR touches the given topic and has a low impact on builders.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

on_idle hook

8 participants