-
Couldn't load subscription status.
- Fork 13
Improve README #208
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
Improve README #208
Conversation
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.
Pull Request Overview
This PR improves the README documentation by enhancing usage examples and adding new sections for better user guidance. The changes focus on replacing broken links with working references, adding authorization documentation, and introducing formula usage examples.
- Replaces broken examples link with a reference to the CLI tool source code
- Adds documentation link for authorization and signing details
- Introduces a new section explaining formula usage with practical examples
README.md
Outdated
| ## Usage | ||
|
|
||
| Please also refer to [examples](https://github.com/frequenz-floss/frequenz-client-reporting-python/tree/HEAD/examples) for more detailed usage. | ||
| Please also refer to source of the [CLI tool](src/frequenz/client/reporting/cli/__main__.py) for a practical example of how to use the client. |
Copilot
AI
Jul 16, 2025
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.
The phrase 'source of the' is grammatically awkward. Consider changing to 'Please also refer to the CLI tool source code for a practical example of how to use the client.'
| Please also refer to source of the [CLI tool](src/frequenz/client/reporting/cli/__main__.py) for a practical example of how to use the client. | |
| Please also refer to the [CLI tool](src/frequenz/client/reporting/cli/__main__.py) source code for a practical example of how to use the client. |
README.md
Outdated
| Note that this endpoint must be used with a `resampling_period`. | ||
| Details on the formula syntax can be found [here](https://github.com/frequenz-floss/frequenz-microgrid-formula-engine-rs/tree/v0.x.x?tab=readme-ov-file#formula-syntax-overview). | ||
|
|
||
| ``` |
Copilot
AI
Jul 16, 2025
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.
The code block is missing a language identifier. Consider adding 'python' after the opening backticks for proper syntax highlighting: ```python
| ``` | |
| ```python |
4ddc60f to
7838e9e
Compare
Signed-off-by: cwasicki <[email protected]>
Signed-off-by: cwasicki <[email protected]>
Signed-off-by: cwasicki <[email protected]>
Signed-off-by: cwasicki <[email protected]>
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.
LGTM
2707a4f
| - Defining a time range with `start_time` and `end_time` | ||
| - Optional downsampling using `resampling_period` (e.g., `timedelta(minutes=15)`) | ||
| - Selecting specific `microgrid_id` and `component_id`. | ||
| - Choosing one or more `metrics` to retrieve. Available metrics are listed [here](https://github.com/frequenz-floss/frequenz-api-common/blob/v0.8.0/proto/frequenz/api/common/v1alpha8/metrics/metrics.proto). |
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.
Not sure if it is much better, but you could also point to the generated docs:
- For pure protobuf: https://frequenz-floss.github.io/frequenz-api-common/v0.8/protobuf-reference/frequenz/api/common/v1alpha8/metrics/metrics/#frequenz-api-common-v1alpha8-metrics-Metric
- For Python it seems the docs are not being properly generated :( https://frequenz-floss.github.io/frequenz-api-common/v0.8/python-reference/frequenz/api/common/v1alpha8/metrics/metrics_pb2/#frequenz.api.common.v1alpha8.metrics.metrics_pb2.Metric
In a near future, we should probably use client-common and that should have a nice Python doc.
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.
Good idea: #209
No description provided.