Skip to content

Add energy metric information#235

Merged
flora-hofmann-frequenz merged 1 commit intofrequenz-floss:v0.x.xfrom
flora-hofmann-frequenz:energy_metric_update
Aug 6, 2024
Merged

Add energy metric information#235
flora-hofmann-frequenz merged 1 commit intofrequenz-floss:v0.x.xfrom
flora-hofmann-frequenz:energy_metric_update

Conversation

@flora-hofmann-frequenz
Copy link
Copy Markdown
Contributor

This is the same additional info as in the common-client.

@flora-hofmann-frequenz flora-hofmann-frequenz requested a review from a team as a code owner August 6, 2024 07:48
@github-actions github-actions Bot added part:docs Affects the documentation part:protobuf Affects the protocol buffer definition files labels Aug 6, 2024
Comment on lines +66 to +79
//
// AC energy metrics information:
//
// * This energy metric is reported directly from the component, and not a
// result of aggregations in our systems. If a component does not have this
// metric, this field cannot be populated.
//
// * Components that provide energy metrics reset this metric from time to
// time. This behaviour is specific to each component model. E.g., some
// components reset it on UTC 00:00:00.
//
// * This energy metric does not specify the timestamp since when the energy
// was being accumulated, and therefore can be inconsistent.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM in general. Should we make it more visible by making it a note?

That way it looks more like a notice, and not general documentation about this enum.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

A bunch of CI test are failing now that I was trying to mark it down as a note. Should I change it back?

Comment on lines +67 to +76
> [!NOTE]
> AC energy metrics information:
> * This energy metric is reported directly from the component, and not a
> result of aggregations in our systems. If a component does not have this
> metric, this field cannot be populated.
> * Components that provide energy metrics reset this metric from time to
> time. This behaviour is specific to each component model. E.g., some
> components reset it on UTC 00:00:00.
> * This energy metric does not specify the timestamp since when the energy
> was being accumulated, and therefore can be inconsistent.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The comment looks incorrect .. these should start wist //, no?

Maybe better to use the MkDocs format here. Here is an example:

// !!! note
//     This is a note.
//     This is the second line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah sorry, I thought it was suppose to look like in the markup docs (but maybe I looked at the wrong one).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MkDocs now supports also GitHub format toob(via a plugin, not sure if this repo is up to date enough to use it though). in any case, using GitHub format probably makes sense only in markdown files that will actually be interpreted by GitHub, which is not the case for .proto files, so I agree it might be better to use MkDocs format instead here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good :-)

Signed-off-by: Flora <flora.hofmann@frequenz.com>
Copy link
Copy Markdown
Contributor

@tiyash-basu-frequenz tiyash-basu-frequenz left a comment

Choose a reason for hiding this comment

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

LGTM!

@flora-hofmann-frequenz flora-hofmann-frequenz added this pull request to the merge queue Aug 6, 2024
Merged via the queue into frequenz-floss:v0.x.x with commit 1c485e4 Aug 6, 2024
@flora-hofmann-frequenz flora-hofmann-frequenz deleted the energy_metric_update branch August 6, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:docs Affects the documentation part:protobuf Affects the protocol buffer definition files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants