-
Notifications
You must be signed in to change notification settings - Fork 808
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pallet view functions: improve metadata, API docs and testing #7412
base: master
Are you sure you want to change the base?
Conversation
All GitHub workflows were cancelled due to failure one of the required jobs. |
/cmd prdoc --audience runtime_dev --bump patch |
|
||
#[pallet::view_functions_experimental] | ||
impl<T: Config> Pallet<T> { | ||
fn get_value() { |
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.
fn get_value() { | |
pub fn get_value() { |
@@ -212,6 +179,8 @@ pub struct PalletMetadataIR<T: Form = MetaForm> { | |||
pub storage: Option<PalletStorageMetadataIR<T>>, | |||
/// Pallet calls metadata. | |||
pub calls: Option<PalletCallMetadataIR<T>>, | |||
/// Pallet view functions metadata. | |||
pub view_functions: Vec<PalletViewFunctionMethodMetadataIR<T>>, |
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.
As a note, moving the view functions to pallet means we can't keep the same view function when pallets are reorganizing, merging or splitting.
@@ -63,6 +63,12 @@ impl From<codec::Error> for ViewFunctionDispatchError { | |||
/// Implemented by both pallets and the runtime. The runtime is dispatching by prefix using the | |||
/// pallet implementation of `ViewFunctionIdPrefix` then the pallet is dispatching by suffix using | |||
/// the methods implementation of `ViewFunctionIdSuffix`. | |||
/// | |||
/// More in details, `ViewFunctionId` = `ViewFunctionIdPrefix` ++ `ViewFunctionIdSuffix`, where |
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.
/// More in details, `ViewFunctionId` = `ViewFunctionIdPrefix` ++ `ViewFunctionIdSuffix`, where | |
/// In more details, `ViewFunctionId` = `ViewFunctionIdPrefix` ++ `ViewFunctionIdSuffix`, where |
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 have no opinion on whether view functions should be defined in the pallets or in the runtime.
Having it in the runtime could allow some more stable view functions for the runtime when pallets are reorganizing/merging/splitting. But at the same time I don't expect people writing runtime to implement the view functions themself in order to maintain the view functions.
Apart from this the code is good to me.
view_functions_experimental
macro topallet_macros
with API docs