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

Rename AllPallets to AllPalletsWithoutSystem and AllPalletsWithSystem to AllPallet #9105

Closed
shawntabrizi opened this issue Jun 14, 2021 · 5 comments · Fixed by #10043
Closed
Labels
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

@shawntabrizi
Copy link
Member

shawntabrizi commented Jun 14, 2021

Let's remove one more footgun.

Where is AllPalletWithoutSystem useful?

cc @thiolliere

@shawntabrizi shawntabrizi added 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. labels Jun 14, 2021
@gui1117
Copy link
Contributor

gui1117 commented Jun 14, 2021

yes, but we have to careful that people don't miss this change, and execute some system stuff 2 times.
Maybe for a better transition we should do: rename AllPalletsWithSystem to AllPallet in another release or find another name which doesn't conflict, like AllPalletsTuple or PalletsTuple while deprecating AllPallets

@KiChjang
Copy link
Contributor

I like to be more pedantic and have AllPalletsWithSystem and AllPalletsWithoutSystem. That way there's no way people can mistake one for the other.

@TomaszWaszczyk
Copy link

The issue relies only on renaming types AllPallets and AllPalletsWithSystem or also some changes in logic? What version of changes should be implemented, version of @shawntabrizi or @KiChjang ?

I look for practical experience in Rust and I am willing to implement simple tasks.

@gui1117
Copy link
Contributor

gui1117 commented Sep 28, 2021

IMO we should do KiChjang suggestion, otherwise people will very likely miss the breaking change.

@gui1117
Copy link
Contributor

gui1117 commented Sep 28, 2021

Or a new name with deprecated old type alias

type AllPalletsTuple = ..
type AllPalletsTypleWithoutSystem = ..
#[deprecated(..)]
type AllPallets = AllPalletsTupleWithoutSystem
#[deprecated(..)]
type AllPalletsWithSystem = AllPalletsTuple

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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
None yet
4 participants