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

configuration: refactor configuration initialization#4569

Merged
paritytech-processbot[bot] merged 1 commit intomasterfrom
pep-configuration-init-refactor
Dec 22, 2021
Merged

configuration: refactor configuration initialization#4569
paritytech-processbot[bot] merged 1 commit intomasterfrom
pep-configuration-init-refactor

Conversation

@pepyakin
Copy link
Copy Markdown
Contributor

Refactor the configuration module's initializer_on_new_session in such a
way that it returns the configuration. This would make it inline with
other special initialization routines like shared's or paras.

This will be useful in a following PR that will check consistency of the
configuration before setting it.

@pepyakin
Copy link
Copy Markdown
Contributor Author

pepyakin commented Dec 20, 2021

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Dec 20, 2021
@pepyakin pepyakin force-pushed the pep-configuration-init-refactor branch from d55fbfb to 0b33c5b Compare December 20, 2021 12:59
@pepyakin pepyakin 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Dec 20, 2021
@pepyakin pepyakin force-pushed the pep-configuration-fixes branch from 4e47095 to 1e40691 Compare December 20, 2021 21:46
Base automatically changed from pep-configuration-fixes to master December 21, 2021 13:28
@pepyakin pepyakin force-pushed the pep-configuration-init-refactor branch from 0b33c5b to 35a806b Compare December 21, 2021 15:17
/// be the same.
pub(crate) fn initializer_on_new_session(
session_index: &SessionIndex,
) -> (HostConfiguration<T::BlockNumber>, HostConfiguration<T::BlockNumber>) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We could make errors way less likely here if we returned a struct with members: old, new or something. A tuple with two identically typed values feels quite error prone.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair, will do.

Refactor the configuration module's initializer_on_new_session in such a
way that it returns the configuration. This would make it inline with
other special initialization routines like `shared`'s or `paras`.

This will be useful in a following PR that will check consistency of the
configuration before setting it.
@ordian
Copy link
Copy Markdown

ordian commented Dec 22, 2021

bot merge

@paritytech-processbot
Copy link
Copy Markdown

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 95a924f into master Dec 22, 2021
@paritytech-processbot paritytech-processbot bot deleted the pep-configuration-init-refactor branch December 22, 2021 16:07
drahnr pushed a commit that referenced this pull request Jan 4, 2022
Refactor the configuration module's initializer_on_new_session in such a
way that it returns the configuration. This would make it inline with
other special initialization routines like `shared`'s or `paras`.

This will be useful in a following PR that will check consistency of the
configuration before setting it.
Wizdave97 pushed a commit to ComposableFi/polkadot that referenced this pull request Feb 3, 2022
Refactor the configuration module's initializer_on_new_session in such a
way that it returns the configuration. This would make it inline with
other special initialization routines like `shared`'s or `paras`.

This will be useful in a following PR that will check consistency of the
configuration before setting it.
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