-
Notifications
You must be signed in to change notification settings - Fork 184
chore: remove displaydoc in favour of derive_more::Display
#6472
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
Conversation
WalkthroughRemoved the Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
053db7c to
c181d76
Compare
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/rpc/reflect/mod.rs (1)
88-112: Add#[display(rename_all = "snake_case")]to match serialization output.The
#[serde(rename_all = "snake_case")]only controls JSON serialization; it doesn't affect theDisplayimplementation. Without an explicit#[display(...)]attribute,derive_more::Displaywill output variant names in PascalCase ("Admin","Sign", etc.). Add the enum-level attribute to ensure consistency.🔧 Suggested fix
#[derive( Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, derive_more::Display, Serialize, Deserialize, )] +#[display(rename_all = "snake_case")] #[serde(rename_all = "snake_case")] pub enum Permission {src/networks/mod.rs (1)
62-76: Add explicit#[display(...)]attributes to NetworkChain variants to preserve lowercase output.The migration from
displaydoctoderive_more::Displayremoved custom formatting. Without explicit#[display(...)]attributes, unit variants will display as capitalized variant names (e.g.,"Mainnet"), breaking thenetwork_chain_displaytest which expects lowercase names (e.g.,"mainnet"). TheDevnet(String)variant will work correctly asderive_moreforwards single-field tuples to the inner type's Display.Suggested fix
pub enum NetworkChain { /// mainnet + #[display("mainnet")] #[default] Mainnet, /// calibnet + #[display("calibnet")] Calibnet, /// butterflynet + #[display("butterflynet")] Butterflynet, /// devnet Devnet(String), }
fd1c9ee to
eb6755f
Compare
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.