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

Conversation

coriolinus
Copy link
Contributor

Closes #1268.

Modeled on this ProvideInherent implementation.

It's not obvious where the data is actually injected into InherentData, but presumably this is the responsibility of the runtime?

@coriolinus coriolinus self-assigned this Jun 26, 2020
@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Jun 26, 2020
@bkchr
Copy link
Member

bkchr commented Jun 26, 2020

The data is not injected in the runtime. You already have implemented an inherent for cumulus and there you also injected the data in the node for the runtime.

It's not clear precisely why this is desired, but it's a pattern
I've seen in several places, so I'm going this to be on the
safe side. Worst case, we can revert this commit pretty easily.
@coriolinus coriolinus 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 Jun 26, 2020
@coriolinus coriolinus marked this pull request as ready for review June 26, 2020 09:36
@github-actions github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Jun 26, 2020
@@ -0,0 +1,7 @@
//! Inclusion Inherent primitives define types and constants which can be imported
Copy link
Contributor

Choose a reason for hiding this comment

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

missing license header

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Should be fixed now.

Copy link
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM except for license header nit

@rphmeier rphmeier added A6-mustntgrumble and removed A0-please_review Pull request needs code review. labels Jun 27, 2020
@rphmeier rphmeier merged commit 7aa95b1 into master Jun 30, 2020
@rphmeier rphmeier deleted the prgn-inclusioninherent-provideinherent branch June 30, 2020 15:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement ProvideInherent for InclusionInherent module
3 participants