Skip to content

Expect metric only as a generic argument in logical meter formulas#31

Open
shsms wants to merge 4 commits intofrequenz-floss:v0.x.xfrom
shsms:turbofish-all-the-functions
Open

Expect metric only as a generic argument in logical meter formulas#31
shsms wants to merge 4 commits intofrequenz-floss:v0.x.xfrom
shsms:turbofish-all-the-functions

Conversation

@shsms
Copy link
Copy Markdown
Collaborator

@shsms shsms commented Mar 31, 2026

No description provided.

@shsms
Copy link
Copy Markdown
Collaborator Author

shsms commented Mar 31, 2026

Based on #30

@shsms shsms force-pushed the turbofish-all-the-functions branch 2 times, most recently from 3452bcf to 5cb4143 Compare March 31, 2026 14:08
@shsms shsms marked this pull request as ready for review March 31, 2026 14:08
@shsms shsms force-pushed the turbofish-all-the-functions branch 2 times, most recently from a0ef4ad to 66250da Compare April 1, 2026 09:41
@shsms shsms requested a review from Copilot April 1, 2026 11:26
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the logical meter API to treat the selected metric purely as a type-level choice (generic parameter) rather than a runtime value, and refactors formula plumbing accordingly.

Changes:

  • Updated LogicalMeterHandle formula streaming methods to take the metric via a generic type argument (removing the metric value parameter).
  • Refactored formula construction to stop carrying metric instances (using PhantomData and associated constants instead).
  • Updated tests, examples, and release notes to match the new call syntax.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/metric.rs Adjusts the Metric trait and metric definitions to support type-only usage (adds str_name(), removes Display impls).
src/logical_meter/logical_meter_handle.rs Changes public handle methods to use metric as a generic argument and updates call sites in tests.
src/logical_meter/formula/graph_formula_provider.rs Removes runtime metric parameters from graph formula provider APIs; updates error message formatting.
src/logical_meter/formula/coalesce_formula.rs Removes stored metric value from formula structs and uses PhantomData.
src/logical_meter/formula/aggregation_formula.rs Removes stored metric value from formula structs and uses PhantomData.
src/logical_meter/formula.rs Updates FormulaParams to no longer store a metric value (uses PhantomData).
RELEASE_NOTES.md Documents the new LogicalMeterHandle call syntax (needs wording + additional breaking-change notes).
examples/logical_meter.rs Updates example usage to the new turbofish syntax.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/metric.rs Outdated
Comment thread src/metric.rs
Comment thread src/logical_meter/logical_meter_handle.rs
Comment thread src/logical_meter/logical_meter_handle.rs
Comment thread RELEASE_NOTES.md Outdated
@shsms shsms force-pushed the turbofish-all-the-functions branch from 66250da to 875ecff Compare April 1, 2026 11:53
shsms added 2 commits April 1, 2026 13:54
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
And no-longer as a function parameter.

Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
@shsms shsms force-pushed the turbofish-all-the-functions branch from 875ecff to 14ee1a9 Compare April 1, 2026 11:55
shsms added 2 commits April 1, 2026 14:10
Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
There is no reason for them to be mutable anymore.

Signed-off-by: Sahas Subramanian <sahas.subramanian@proton.me>
@shsms shsms force-pushed the turbofish-all-the-functions branch from de1b3cb to 1404c90 Compare April 1, 2026 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants