Skip to content
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 macro: Support instantiable pallets in tasks #5194

Merged
merged 24 commits into from
Oct 24, 2024

Conversation

gui1117
Copy link
Contributor

@gui1117 gui1117 commented Jul 31, 2024

Fix #5185

also implement handling of attr in expansion in construct-runtime

@gui1117 gui1117 requested a review from a team as a code owner July 31, 2024 02:13
pub fn expand_tasks(def: &mut Def) -> TokenStream2 {
if let Some(tasks_def) = &def.tasks {
if def.task_enum.is_none() {
def.task_enum = Some(TaskEnumDef::generate(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this PR we no longer modify the Def to insert a generated task_enum in case none were provided.

Instead we just expand what we need without modifying the Def in the expand phase.

As a result we need to use tasks instead of task_enum for generating the automatic part later.

But all in all behavior is not changed

@ggwpez ggwpez requested a review from gupnik August 10, 2024 14:05
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

LGTM but am not deep enough into the code.

@gui1117 gui1117 added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Sep 9, 2024
@Doordashcon
Copy link
Contributor

More eyes on this 🙏

let ident = &self.item_enum.ident;
let vis = &self.item_enum.vis;
let attrs = &self.item_enum.attrs;
let generics = &self.item_enum.generics;
Copy link
Member

Choose a reason for hiding this comment

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

These should be the same as type_use_generics, if not we would have some problem. I think we should remove generics and just use the type_use_generics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a check for generics at eb3061c

@gui1117 gui1117 added this pull request to the merge queue Oct 24, 2024
Merged via the queue into paritytech:master with commit 68af85e Oct 24, 2024
196 checks passed
@gui1117 gui1117 deleted the gui-tasks-instantiable-pallets branch October 24, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pallet macro: Tasks doesn't support instantiable pallets.
4 participants