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

Support extra constant renaming #9814

Merged
12 commits merged into from
Oct 7, 2021
Merged

Support extra constant renaming #9814

12 commits merged into from
Oct 7, 2021

Conversation

ericjmiller
Copy link
Contributor

@ericjmiller ericjmiller commented Sep 18, 2021 •
edited
Loading

#8826 Pallet macro: support pallet::extra_constants

Allow renaming of constants with a procedural macro. This will enable the convention of snake cased methods, but camel cased constant in the metadata. e.g.

#[pallet::extra_constant]
impl<T: Config> Pallet<T> {

    #[pallet::constant_name(FooBar)]
    fn foo_bar() -> u32 {..}

}

Kusama address: HqQ5BKxXgcLB2yaoSzj383xsFF4tYTUwnhZB12zvfQeEx2N

@thiolliere
Copy link
Contributor

syntax looks good to me

@kianenigma kianenigma added the A3-in_progress Pull request is in progress. No review needed at this stage. label Sep 20, 2021
@ericjmiller ericjmiller marked this pull request as ready for review September 22, 2021 23:35
thiolliere
thiolliere approved these changes Sep 23, 2021
View reviewed changes
Copy link
Contributor

@thiolliere thiolliere left a comment

Choose a reason for hiding this comment

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

can be simplified, but logic is good to me.

frame/support/procedural/src/pallet/parse/extra_constants.rs Outdated Show resolved Hide resolved
frame/support/procedural/src/pallet/expand/constants.rs Outdated Show resolved Hide resolved
frame/support/procedural/src/pallet/parse/extra_constants.rs Show resolved Hide resolved
@thiolliere thiolliere added A0-please_review Pull request needs code review. B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Sep 27, 2021
@thiolliere thiolliere added this to In progress in Runtime via automation Sep 27, 2021
@thiolliere thiolliere moved this from In progress to Please Review in Runtime Sep 27, 2021
@thiolliere
Copy link
Contributor

some minor change needs to be done to make CI happy

@shawntabrizi
Copy link
Member

someone needs to fix compile

@ericjmiller
Copy link
Contributor Author

Sorry - I'll fix. Had an intense sprint at work and was strapped for time.

@ericjmiller
Copy link
Contributor Author

Okay, I rebased with master and suddenly 342 files have been changed?

@ericjmiller ericjmiller force-pushed the support-extra_constant-renaming branch from 69acf1c to a4bdad4 Compare October 7, 2021 18:04
Eric Miller added 6 commits October 7, 2021 15:01
Rebased with master. Resolved merge conflict in
d9363bb 
frame/support/test/tests/pallet.rs
Switching Account ID to SomeType1, as SomeType3 was giving me convers…
d6ec83e 
…ion error.
Wrong indent config. Fixed.
e7c5a63
These tabs look fine locally, but look different on Github. Trying to…
60edcac 
… get the style config right.
Parsing pallet::constant_name.
275b2a1 
Passing unit tests, which is confusing because I didn't change `ident` in the ExtraConstantDef initialization.
Finalized parsing of extra constant name by adding optional metadata …
98043ff 
…field.

Added expansion logic that replaces respective `idents` where they exist.
Eric Miller and others added 4 commits October 7, 2021 15:05
Erasing this to try to keep the format the same across the source code.
e0b10eb
Another formatting change for consistency.
e6e4e55
@ericjmiller @thiolliere
Update frame/support/procedural/src/pallet/expand/constants.rs
3b6ef1c 
strictly more idiomatic.

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
@ericjmiller @thiolliere
Update frame/support/procedural/src/pallet/parse/extra_constants.rs
bee44a5 
strictly idiomatic change.

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
@ericjmiller ericjmiller force-pushed the support-extra_constant-renaming branch from a4bdad4 to bee44a5 Compare October 7, 2021 19:06
Runtime automation moved this from Please Review to Needs Audit Oct 7, 2021
@shawntabrizi
Copy link
Member

/tip small

@substrate-tip-bot
Copy link

Please fix the following problems before calling the tip bot again:

  • Contributor did not properly post their Polkadot or Kusama address. Make sure the pull request has: "{network} address: {address}".

@ericjmiller
Copy link
Contributor Author

/tip small

Didn't know that was a "thing". Updated PR! <3

@shawntabrizi
Copy link
Member

/tip small

@substrate-tip-bot
Copy link

A small tip was successfully submitted for ericjmiller (HqQ5BKxXgcLB2yaoSzj383xsFF4tYTUwnhZB12zvfQeEx2N on kusama).

https://polkadot.js.org/apps/#/treasury/tips

@shawntabrizi
Copy link
Member

bot merge

@ghost
Copy link

ghost commented Oct 7, 2021

Trying merge.

@ghost ghost merged commit 48f3cdd into paritytech:master Oct 7, 2021
Runtime automation moved this from Needs Audit to Done Oct 7, 2021
@github-actions github-actions bot mentioned this pull request Nov 2, 2021
Closed
@shawntabrizi shawntabrizi moved this from Done to Archive in Runtime Nov 11, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@shawntabrizi shawntabrizi shawntabrizi approved these changes

@thiolliere thiolliere thiolliere approved these changes

@adoerr adoerr Awaiting requested review from adoerr

@andresilva andresilva Awaiting requested review from andresilva

@athei athei Awaiting requested review from athei

@kianenigma kianenigma Awaiting requested review from kianenigma

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ericjmiller @thiolliere @shawntabrizi @kianenigma