Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Rename outer enums to RuntimeCall, RuntimeEvent, etc #11736

Closed
3 tasks done
kianenigma opened this issue Jun 22, 2022 · 16 comments
Closed
3 tasks done

Rename outer enums to RuntimeCall, RuntimeEvent, etc #11736

kianenigma opened this issue Jun 22, 2022 · 16 comments
Labels
I7-refactor Code needs refactoring. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder Z6-mentor An easy task where a mentor is available. Please indicate in the issue who the mentor could be.

Comments

@kianenigma
Copy link
Contributor

kianenigma commented Jun 22, 2022

we should really just rename the Call generated by the construct_runtime! to RuntimeCall.

Originally posted by @kianenigma in #8458 (comment)

  • Call
  • Event
  • Origin

See confusions such as https://substrate.stackexchange.com/questions/3360/using-scheduler-pallet-to-schedule-contract-pallet-call

@kianenigma kianenigma added the I7-refactor Code needs refactoring. label Jun 22, 2022
@kianenigma kianenigma added Z6-mentor An easy task where a mentor is available. Please indicate in the issue who the mentor could be. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder and removed Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Jun 22, 2022
@KiChjang
Copy link
Contributor

As mentioned in the PR, I don't really like the idea of naming it OuterCall, as it implies that there's an "inner" call, and it's not obvious what it is.

However, based on discussions, we have settled upon naming them RuntimeCalls and PalletCalls. These two names should be self-explanatory about what they refer to.

@tifecool
Copy link
Contributor

I'd like to pick up this issue. Would I have to work in-depth with macros?

@shawntabrizi
Copy link
Member

shawntabrizi commented Jun 29, 2022

@tifecool yes you can pick it up

I don't think you will need to work deeply, but you will need to adjust some of the enum names generated by the macros.

Ideally, we could support the old enum name with some backwards compatibility config

@shawntabrizi shawntabrizi changed the title Rename outer enums to OuterCall, OuterEvent, etc Rename outer enums to RuntimeCall, RuntimeEvent, etc Jul 30, 2022
@kianenigma
Copy link
Contributor Author

@KiChjang will you be working on this?

@KiChjang
Copy link
Contributor

KiChjang commented Aug 2, 2022

I could if it's urgent, otherwise other people can pick this up if they're interested.

@kianenigma
Copy link
Contributor Author

yeah was just wondering since we talked about it in person. Nah, I don't think it is urgent and it is a good first-issue for other contributors to FRAME.

@tifecool
Copy link
Contributor

tifecool commented Aug 2, 2022

Couldn't figure out where to change the Calls. I don't mind someone else picking the issue up. Would be nice to see the solution

Edit: Still attempting, but if someone else takes it it's fine.

@Szegoo
Copy link
Contributor

Szegoo commented Aug 4, 2022

Couldn't figure out where to change the Calls. I don't mind someone else picking the issue up. Would be nice to see the solution

Edit: Still attempting, but if someone else takes it it's fine.

Have you managed to get it working? If not I will probably make a PR. I believe that the files that need to be modified are: frame/support/procedural/src/construct_runtime/expand/call.rs and frame/support/procedural/src/construct_runtime/expand/event.rs

@tifecool
Copy link
Contributor

tifecool commented Aug 4, 2022

I believe that the files that need to be modified are: frame/support/procedural/src/construct_runtime/expand/call.rs and frame/support/procedural/src/construct_runtime/expand/event.rs

Hey @Szegoo. Funny enough just figured that out yesterday hence the "Still attempting" edit. Now I'm getting the error:
error[E0433]: failed to resolve: use of undeclared type "Call"

Dropping the issue now. Please go ahead 😇️

Edit: If you run into the same issue would love to know how you worked it out. I think having a way to see where exactly the macro tries to use "Call" would be very helpful.

@Szegoo
Copy link
Contributor

Szegoo commented Aug 4, 2022

@tifecool Sorry, I didn't notice the "Still attempting" edit. Feel free to continue working on this if you want to, I didn't want to make any pressure on you. If you really want to drop this issue, I would be happy to work on this.

@tifecool
Copy link
Contributor

tifecool commented Aug 4, 2022

@Szegoo It's no pressure. You're welcome to work on it. I'll learn from your solution.

@kianenigma
Copy link
Contributor Author

We could probably use a small PR for origin at some point too?

@Szegoo
Copy link
Contributor

Szegoo commented Sep 13, 2022

We could probably use a small PR for origin at some point too?

I will open a new PR for that.

@Szegoo
Copy link
Contributor

Szegoo commented Sep 24, 2022

Shouldn't this issue be closed?

@KiChjang
Copy link
Contributor

Yes.

@shredding
Copy link

Please update the documentation as well. It'll save the next dev after me hours of frustration.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I7-refactor Code needs refactoring. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder Z6-mentor An easy task where a mentor is available. Please indicate in the issue who the mentor could be.
Projects
Status: Done
Development

No branches or pull requests

6 participants