Skip to content

Conversation

@dimlio
Copy link
Collaborator

@dimlio dimlio commented Feb 14, 2023

Description

Align output of histogram percentile distribution with upstream Java implementation.

How Has This Been Tested?

Added unit tests.

Minimal checklist:

  • I have performed a self-review of my own code
  • I have added DocC code-level documentation for any public interfaces exported by the package
  • I have added unit and/or integration tests that prove my fix is effective or that my feature works

Can happen when recording value above highest trackable to non-autoresizing
histogram.
Differs from 'percentile' field for percentile iteration.
@dimlio dimlio requested a review from hassila February 14, 2023 16:24
@codecov
Copy link

codecov bot commented Feb 14, 2023

Codecov Report

Merging #5 (01cfc97) into main (f1b50a2) will increase coverage by 5.38%.
The diff coverage is 98.20%.

❗ Current head 01cfc97 differs from pull request most recent head 760aac6. Consider uploading reports for the commit 760aac6 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main       #5      +/-   ##
==========================================
+ Coverage   92.60%   97.98%   +5.38%     
==========================================
  Files           5        4       -1     
  Lines        1540     1631      +91     
==========================================
+ Hits         1426     1598     +172     
+ Misses        114       33      -81     
Impacted Files Coverage Δ
...ests/HistogramTests/HistogramAutosizingTests.swift 96.92% <0.00%> (ø)
...ests/HistogramTests/HistogramDataAccessTests.swift 99.07% <96.88%> (+1.26%) ⬆️
Sources/Histogram/Histogram.swift 96.08% <98.51%> (+8.91%) ⬆️
Tests/HistogramTests/HistogramTests.swift 100.00% <100.00%> (ø)
Impacted Files Coverage Δ
...ests/HistogramTests/HistogramAutosizingTests.swift 96.92% <0.00%> (ø)
...ests/HistogramTests/HistogramDataAccessTests.swift 99.07% <96.88%> (+1.26%) ⬆️
Sources/Histogram/Histogram.swift 96.08% <98.51%> (+8.91%) ⬆️
Tests/HistogramTests/HistogramTests.swift 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f1b50a2...760aac6. Read the comment docs.

@hassila
Copy link
Contributor

hassila commented Feb 14, 2023

Thanks, LGTM, will give it a spin tomorrow - did this sort out the decimal issue too in the output ("000")? (Also some swiftlint cruft - but lets fix that separately)

@dimlio dimlio force-pushed the align-log-format-with-upstream-implementation branch from 2917817 to 5a88631 Compare February 14, 2023 17:07
@hassila
Copy link
Contributor

hassila commented Feb 15, 2023

Still got "000" - open a different issue for that, or within the context of this one?

Copy link
Collaborator Author

dimlio commented Feb 15, 2023

well, I made output identical to Java version

@dimlio dimlio force-pushed the align-log-format-with-upstream-implementation branch from 5a88631 to a4fbd3a Compare February 15, 2023 11:33
@dimlio dimlio force-pushed the align-log-format-with-upstream-implementation branch from a4fbd3a to 01cfc97 Compare February 15, 2023 11:40
@dimlio dimlio force-pushed the align-log-format-with-upstream-implementation branch from 01cfc97 to 760aac6 Compare February 15, 2023 11:47
@dimlio dimlio merged commit 49df02b into main Feb 15, 2023
@dimlio dimlio deleted the align-log-format-with-upstream-implementation branch February 15, 2023 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants