Skip to content

Fix/harmonizing timestamps#113

Merged
bpetit merged 21 commits intomainfrom
fix/harmonizing-timestamps
Oct 5, 2021
Merged

Fix/harmonizing timestamps#113
bpetit merged 21 commits intomainfrom
fix/harmonizing-timestamps

Conversation

@bpetit
Copy link
Copy Markdown
Contributor

@bpetit bpetit commented May 14, 2021

No description provided.

@bpetit bpetit requested a review from PierreRust May 14, 2021 16:16
@bpetit bpetit linked an issue May 14, 2021 that may be closed by this pull request
@PierreRust
Copy link
Copy Markdown
Collaborator

Looks fine to me. To get timestamp in the json exporter we should probably use the MetricGenerator in that exporter too (and the stdout too, for consistency) . Ideally that should be done before merging this PR as it might induce some changes. Should I implement this in this PR or in another one ?

@bpetit bpetit added this to the Release v0.4.0 milestone Sep 11, 2021
@bpetit
Copy link
Copy Markdown
Contributor Author

bpetit commented Sep 22, 2021

Hi @PierreRust ! I've started refactoring the stdout and json exporters to use MetricGenerator.
Stdout is somehow done (could be improved but uses MetricGenerator anyway).
Json on the way.

I'd love to read your thoughts about the code !

Once done we'll be able to merge this PR ! 🥳

Copy link
Copy Markdown
Contributor Author

@bpetit bpetit left a comment

Choose a reason for hiding this comment

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

first review, waiting for another one from someone else

@bpetit bpetit requested a review from uggla September 30, 2021 14:45
@bpetit bpetit linked an issue Oct 1, 2021 that may be closed by this pull request
@bpetit bpetit merged commit e950fe2 into main Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Previous releases

Development

Successfully merging this pull request may close these issues.

Shortcomings of current json exporter

2 participants