Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Adds instance support for composite enums #1857

Merged
merged 5 commits into from
Oct 13, 2023
Merged

Adds instance support for composite enums #1857

merged 5 commits into from
Oct 13, 2023

Conversation

gupnik
Copy link
Contributor

@gupnik gupnik commented Oct 12, 2023 •
edited
Loading

Fixes #1839

Currently, composite_enums do not support pallet instances. This PR allows the following:

	#[pallet::composite_enum]
	pub enum HoldReason<I: 'static = ()> {
		SomeHoldReason
	}

Todo

  • UI Test

@gupnik gupnik added R0-silent Changes should not be mentioned in any release notes T1-FRAME This PR/Issue is related to core FRAME, the framework. labels Oct 12, 2023
@gupnik gupnik requested review from a team October 12, 2023 05:23
sam0x17
sam0x17 approved these changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@sam0x17 sam0x17 left a comment

Choose a reason for hiding this comment

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

looks fine to me... update docs? change / add a UI test?

@gupnik
Copy link
Contributor Author

gupnik commented Oct 12, 2023

looks fine to me... update docs? change / add a UI test?

Yeah, have a Todo added :)

bkchr
bkchr approved these changes Oct 12, 2023
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

As a follow up we should create a warning for every type that doesn't has an instance. Let's assume your pallet is instanced, but you forget it somewhere. If this is happening, we should create a warning as otherwise the user will see some weird errors.

@gupnik gupnik mentioned this pull request Oct 12, 2023
Open
@gupnik
Copy link
Contributor Author

gupnik commented Oct 12, 2023

As a follow up we should create a warning for every type that doesn't has an instance. Let's assume your pallet is instanced, but you forget it somewhere. If this is happening, we should create a warning as otherwise the user will see some weird errors.

Makes sense. Created an issue for this: #1858

@gupnik gupnik enabled auto-merge (squash) October 13, 2023 05:04
@gupnik gupnik merged commit 6b27dad into master Oct 13, 2023
@gupnik gupnik deleted the gupnik/composite-enum-fix branch October 13, 2023 05:48
ordian added a commit that referenced this pull request Oct 16, 2023
@ordian
Merge branch 'master' into tsv-disabling-backing
c44093d 
* master: (54 commits)
  Publish `xcm-emulator` crate (#1881)
  Adding migrations to clean Rococo Gov 1 storage & reserved funds (#1849)
  Arkworks Elliptic Curve utils overhaul (#1870)
  Fix typos (#1878)
  fix: GoAhead signal only set when runtime upgrade is enacted from parachain side (#1176)
  Refactor staking ledger (#1484)
  Paired-key Crypto Scheme (#1705)
  Include polkadot version in artifact path (#1828)
  add link to rfc-0001 in broker README (#1862)
  Discard `Executor` (#1855)
  Macros to use path instead of ident (#1474)
  Remove clippy clone-double-ref lint noise (#1860)
  Refactor alliance benchmarks to v2 (#1868)
  Check executor params coherence (#1774)
  frame: use derive-impl for beefy and mmr pallets (#1867)
  sc-consensus-beefy: improve gossip logic (#1852)
  Adds instance support for composite enums (#1857)
  Fix links to implementers' guide (#1865)
  Disabled validators runtime API (#1257)
  Adding `try_state` hook for `Treasury` pallet (#1820)
  ...
ordian added a commit that referenced this pull request Oct 16, 2023
@ordian
Merge branch 'tsv-disabling-backing' into ao-disabling-statement-dist…
b67c19f 
…ribution

* tsv-disabling-backing: (54 commits)
  Publish `xcm-emulator` crate (#1881)
  Adding migrations to clean Rococo Gov 1 storage & reserved funds (#1849)
  Arkworks Elliptic Curve utils overhaul (#1870)
  Fix typos (#1878)
  fix: GoAhead signal only set when runtime upgrade is enacted from parachain side (#1176)
  Refactor staking ledger (#1484)
  Paired-key Crypto Scheme (#1705)
  Include polkadot version in artifact path (#1828)
  add link to rfc-0001 in broker README (#1862)
  Discard `Executor` (#1855)
  Macros to use path instead of ident (#1474)
  Remove clippy clone-double-ref lint noise (#1860)
  Refactor alliance benchmarks to v2 (#1868)
  Check executor params coherence (#1774)
  frame: use derive-impl for beefy and mmr pallets (#1867)
  sc-consensus-beefy: improve gossip logic (#1852)
  Adds instance support for composite enums (#1857)
  Fix links to implementers' guide (#1865)
  Disabled validators runtime API (#1257)
  Adding `try_state` hook for `Treasury` pallet (#1820)
  ...
@kiltbot kiltbot mentioned this pull request Nov 7, 2023
Closed
@ahmadkaouk ahmadkaouk mentioned this pull request Nov 16, 2023
Closed
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
@gupnik
Adds instance support for composite enums (paritytech#1857)
8b127b1 
Fixes paritytech#1839

Currently, `composite_enum`s do not support pallet instances. This PR
allows the following:
```rust
	#[pallet::composite_enum]
	pub enum HoldReason<I: 'static = ()> {
		SomeHoldReason
	}
```

### Todo

- [x]  UI Test
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@sam0x17 sam0x17 sam0x17 approved these changes

@kianenigma kianenigma kianenigma approved these changes

@bkchr bkchr bkchr approved these changes

Assignees
No one assigned
Labels
R0-silent Changes should not be mentioned in any release notes T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HoldReason composite enum in instatiable pallets
4 participants
@gupnik @sam0x17 @kianenigma @bkchr