-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Conversation
#[pallet::composite_enum] | ||
pub enum HoldReason { | ||
/// The NIS Pallet has reserved it for a non-fungible receipt. | ||
#[codec(index = 0)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary, but it's there to ensure that the codec index remains unchanged by being explicit.
@@ -35,6 +35,7 @@ pub fn expand_outer_freeze_reason(pallet_decls: &[Pallet], scrate: &TokenStream) | |||
} | |||
|
|||
quote! { | |||
/// A reason for placing a freeze on funds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to forward the real docs put on top of the type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that what this is already doing? It's documenting a type that is expanded by the proc macro.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't forwarding the docs that I as a user put onto the type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay, so the problem is that RuntimeFreezeReason
is never exposed to the user -- it's generated by construct_runtime
. The only way that this would make sense is if we forward the doc comments on the pallet's FreezeReason
, which I think is already what we do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh okay, missed that this is the runtime level declaration.
bot merge |
* Add HoldReason to the NIS pallet * Rename composable_enum to composite_enum * Add encoding test * Add more doc comments
* Add HoldReason to the NIS pallet * Rename composable_enum to composite_enum * Add encoding test * Add more doc comments
Partial paritytech/polkadot-sdk#236.
Adds the
HoldReason
as acomposite_enum
to the NIS pallet. Also fixes a couple of naming bugs and adds more tests.