-
Notifications
You must be signed in to change notification settings - Fork 36
Add docs for version 2.0 #100
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
- Removed overly verbose issue and pr templates - Removed documentation from README - Readme now links to spi documentation page - Added docc documentation - Removed last usages of `TimeHistogram`
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.
Looks good. Just a couple of small nits
|
||
## Installation | ||
|
||
``Prometheus`` is available through Swift Package Manager. To include it in your project add the |
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.
``Prometheus`` is available through Swift Package Manager. To include it in your project add the | |
``Prometheus`` is available through the Swift Package Manager. To include it in your project add the |
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 "the" is needed here.
|
||
The same functionality is also available for ``ValueHistogram`` and aggregating `Recorder`s. | ||
|
||
[Swift Metrics]: https://github.com/apple/swift-metrics |
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.
Why this link here at the bottom?
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.
this is a markdown feature that allows you to have all the links in the end of your doc. this way the links don't polute the markdown reading experience inline.
Co-authored-by: Franz Busch <[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.
Looks ok, thx
/// to overwrite the Metric names in third party packages. | ||
public var labelAndDimensionSanitizer: @Sendable (_ label: String, _ dimensions: [(String, String)]) -> (String, [(String, String)]) | ||
public var nameAndLabelSanitizer: @Sendable (_ label: String, _ dimensions: [(String, String)]) -> (String, [(String, String)]) |
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 var is called nameAndLabelSanitizer but the parameters are label, dimensions
this is maybe inconsistent
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.
this is a bit confusing re Prometheus and Swift Metrics naming.
Swift Metrics -> Prometheus
label -> name
dimensions -> labels
myGauge.decrement() | ||
``` | ||
|
||
Lastly, you can use your ``PrometheusCollectorRegistry`` to generate a Prometheus export as in the |
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 what 'as in' means here
Co-authored-by: hamzahrmalik <[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
TimeHistogram