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

Soft deprecate GenesisConfig#14210

Merged
paritytech-processbot[bot] merged 11 commits intomasterfrom
jg/14065-deprecation-genesisconfig-runtimegenesisconfig
May 26, 2023
Merged

Soft deprecate GenesisConfig#14210
paritytech-processbot[bot] merged 11 commits intomasterfrom
jg/14065-deprecation-genesisconfig-runtimegenesisconfig

Conversation

@juangirini
Copy link
Contributor

@juangirini juangirini commented May 24, 2023 •
edited
Loading

Relates to paritytech/polkadot-sdk#175

This PR Soft deprecates the runtime level GenesisConfig by renaming it to RuntimeGenesisConfig.
An alias GenesisConfig is created to keep backwards compatibility. No deprecation warning has been added at this stage as that will happen later in a separate PR as indicated in paritytech/polkadot-sdk#175

@juangirini juangirini mentioned this pull request May 24, 2023
Closed
15 tasks
@juangirini juangirini 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. labels May 24, 2023
@juangirini juangirini changed the title [WIP] soft deprecate genesisconfig Soft deprecate genesisconfig May 24, 2023
@juangirini juangirini changed the title Soft deprecate genesisconfig Soft deprecate GenesisConfig May 24, 2023
@juangirini juangirini added the T1-runtime This PR/Issue is related to the topic “runtime”. label May 24, 2023
@juangirini juangirini marked this pull request as ready for review May 24, 2023 09:29
@juangirini juangirini requested a review from a team May 24, 2023 09:29
@kianenigma kianenigma mentioned this pull request Aug 24, 2023
Closed
@juangirini juangirini added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label May 24, 2023
ggwpez
ggwpez reviewed May 24, 2023
View reviewed changes
frame/support/procedural/src/construct_runtime/expand/config.rs Outdated
}

// TODO [#14065]
// - deprecate it
Copy link
Member

@ggwpez ggwpez May 24, 2023

Choose a reason for hiding this comment

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

Could you maybe temporarily put a deprecation here, to see that the CI stays green and we got all cases?

Copy link
Contributor Author

@juangirini juangirini May 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Sure, that's what I did locally. Let's double check with the CI, I will remove it after being confirmed

Copy link
Contributor Author

@juangirini juangirini May 24, 2023

Choose a reason for hiding this comment

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

@ggwpez all green now with the deprecation attribute in place, except for the Cumulus and Polkadot companions of course, as they are throwing the deprecation warning.

Copy link
Member

@ggwpez ggwpez May 24, 2023

Choose a reason for hiding this comment

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

Yes you can do them lean without companion (since that is annoying).

@paritytech-ci paritytech-ci requested a review from a team May 24, 2023 09:35
@juangirini juangirini requested a review from a team May 24, 2023 12:54
@juangirini juangirini requested a review from ggwpez May 24, 2023 14:45
This was referenced May 25, 2023
Merged
Merged
@juangirini
Copy link
Contributor Author

juangirini commented May 25, 2023

bot rebase

@paritytech-processbot
Copy link

paritytech-processbot bot commented May 25, 2023

Rebased

@juangirini
Copy link
Contributor Author

juangirini commented May 25, 2023

bot merge

@paritytech-processbot
Copy link

paritytech-processbot bot commented May 25, 2023

Waiting for commit status.

@paritytech-processbot
Copy link

paritytech-processbot bot commented May 25, 2023

Merge cancelled due to error. Error: Statuses failed for 8121a6f

liamaharon
liamaharon approved these changes May 26, 2023
View reviewed changes
frame/support/procedural/src/construct_runtime/expand/config.rs Outdated
Comment on lines +81 to +83
// TODO [#14065]
// - deprecate it
// - later, remove it
Copy link
Contributor

@liamaharon liamaharon May 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Personally I think it would be cleaner to not put TODO or deprecation steps in the code comment itself, and just list them in the issue.

The code comment could say something like "GenesisConfig is in the process of being deprecated [issue link]"

@juangirini
Copy link
Contributor Author

juangirini commented May 26, 2023

bot merge

@paritytech-processbot
Copy link

paritytech-processbot bot commented May 26, 2023

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 5b255b2 into master May 26, 2023
@paritytech-processbot paritytech-processbot bot deleted the jg/14065-deprecation-genesisconfig-runtimegenesisconfig branch May 26, 2023 09:42
@nazar-pc nazar-pc mentioned this pull request Jun 16, 2023
Merged
1 task
Ank4n pushed a commit that referenced this pull request Jul 8, 2023
@juangirini @Ank4n
Soft deprecate GenesisConfig (#14210)
859946a 
* soft deprecate genesisconfig

* temporarily add a deprecation attr

* update tests

* update mocks

* update genesis config

* update genesis config

* update genesis config

* update genesis config

* remove deprecation warning

* update deprecation comment

---------

Co-authored-by: parity-processbot <>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@juangirini @nathanwhit
Soft deprecate GenesisConfig (paritytech#14210)
3810f8a 
* soft deprecate genesisconfig

* temporarily add a deprecation attr

* update tests

* update mocks

* update genesis config

* update genesis config

* update genesis config

* update genesis config

* remove deprecation warning

* update deprecation comment

---------

Co-authored-by: parity-processbot <>
@crystalin crystalin mentioned this pull request Oct 20, 2023
Closed
@lrazovic lrazovic mentioned this pull request Dec 7, 2023
Merged
@dnjscksdn98 dnjscksdn98 mentioned this pull request Jan 12, 2024
Merged
13 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

5 more reviewers

@ggwpez ggwpez ggwpez approved these changes

@sam0x17 sam0x17 sam0x17 approved these changes

@bkchr bkchr bkchr approved these changes

@muharem muharem muharem approved these changes

@liamaharon liamaharon liamaharon approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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 T1-runtime This PR/Issue is related to the topic “runtime”.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@juangirini @sam0x17 @bkchr @muharem @ggwpez @liamaharon