-
Notifications
You must be signed in to change notification settings - Fork 2.6k
PR on top of PR: kiz frame api #14410
PR on top of PR: kiz frame api #14410
Conversation
The CI pipeline was cancelled due to failure one of the required jobs. |
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.
lgtm!
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.
have some further suggestions, bit happy to merge as is for improving tt_call stuff, and making frame benchmarking macros work with frame-api.
frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../../benchmarking" } | ||
frame-support = { version = "4.0.0-dev", default-features = false, path = "../../support" } | ||
frame-system = { version = "4.0.0-dev", default-features = false, path = "../../system" } | ||
frame = { version = "0.0.1-dev", default-features = false, path = "../../../frame/" } |
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.
If we are coming this far, should we refactor the whole example to also not use sp-* as well? everything should be re-exported from frame.
Or else, we can leave pallet-example-basic as is for now, and add benchmarking to pallet-example-frame-api.
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.
Or else, we can leave pallet-example-basic as is for now, and add benchmarking to pallet-example-frame-api.
maybe it is better to leave pallet-example-basic as is for now, I used it only as a tests, if we move basic example to frame we shouldn't use the re-export frame::deps
so much also, it needs to be cleaned
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.
Agreed, then let's do all experiments in pallet-example-frame-api
for now.
@@ -21,8 +21,8 @@ | |||
#![cfg(feature = "runtime-benchmarks")] | |||
|
|||
use crate::*; | |||
use frame_benchmarking::v2::*; | |||
use frame_system::RawOrigin; | |||
use frame::deps::frame_benchmarking::v2::*; |
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.
would be good to have a benchmarking_prelude in frame crate now.
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.
Yes I agree
this PR was open as an example |
on top of #14137
cc @kianenigma
with this PR pallet-example-basic compiles without frame_benchmarking, frame_system and frame_support dependency.
Code is not well written, but this is enough for ensuring that it works
For the construct_runtime/tt_default_parts, I reverted the change so that path to
frame_support
to passed along.For the benchmarking macro, only system crate needed to be abstracted