Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Switch float representation to OCaml's default "%f" #22

Merged
merged 5 commits into from
Feb 26, 2020
Merged

Switch float representation to OCaml's default "%f" #22

merged 5 commits into from
Feb 26, 2020

Conversation

toots
Copy link
Contributor

@toots toots commented Dec 3, 2019

The default float representation from Fmt.float uses a scientific notation that looses significant information on data such as timestamps:

  Fmt.strf "%a" Fmt.float 1575363850.57 --> 1.57536e+09

toots
toots commented Dec 3, 2019
View reviewed changes
CHANGES.md Outdated Show resolved Hide resolved
toots
toots commented Dec 3, 2019
View reviewed changes
app/prometheus_app.ml Outdated Show resolved Hide resolved
avsm
avsm approved these changes Jan 1, 2020
View reviewed changes
Copy link
Member

@avsm avsm left a comment

Choose a reason for hiding this comment

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

This lgtm! Will merge/release later in the week when back properly.

app/prometheus_app.ml Outdated Show resolved Hide resolved
@toots @avsm
Update app/prometheus_app.ml
a561111 
Co-Authored-By: Anil Madhavapeddy <anil@recoil.org>
@toots
Copy link
Contributor Author

toots commented Jan 8, 2020

Sounds good, thank you!

@talex5 talex5 merged commit 971ae57 into mirage:master Feb 26, 2020
talex5 added a commit to talex5/opam-repository that referenced this pull request Mar 3, 2020
@talex5
[new release] prometheus and prometheus-app (0.7)
43077e2 
CHANGES:

- switch float representation to OCaml's default `"%f"` (mirage/prometheus#22, @toots)
- use `Gc.quick_stat` for faster stats (mirage/prometheus#25, @talex5)
@talex5 talex5 mentioned this pull request Mar 3, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avsm avsm avsm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@toots @avsm @talex5