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.

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

fix: Add missing tag in Influx datatype header #197

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

clackary
Copy link
Contributor

@clackary clackary commented Nov 9, 2023

The datatype header should have 14 values to match the 14 column headers.

Description

When working with the .csv file produced by --format influx flag, I noticed that a few datatype headers were misaligned with their corresponding columns, and the time column was missing a datatype header entirely. The list of datatype headers seems to be missing one instance of tag.

Actual behavior

tag tag tag tag tag tag tag tag double double long long dateTime
measurement hostName processoryType processors memory kernelVersion metric unit test value test_average iterations warmup_iterations time

Expected behavior

tag tag tag tag tag tag tag tag tag double double long long dateTime
measurement hostName processoryType processors memory kernelVersion metric unit test value test_average iterations warmup_iterations time

How Has This Been Tested?

Run swift package --disable-sandbox --allow-writing-to-package-directory benchmark --format influx.
Verify datatype header aligns with column headers.

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

The datatype header should have 14 values to match the 14 column headers.
@clackary clackary changed the title Add missing tag in Influx datatype header fix: Add missing tag in Influx datatype header Nov 9, 2023
@hassila
Copy link
Contributor

hassila commented Nov 10, 2023

@ORyanHampton ping, you didn’t run into this?

Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Merging #197 (f564019) into main (070dee3) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #197   +/-   ##
=======================================
  Coverage   70.83%   70.83%           
=======================================
  Files          29       29           
  Lines        3685     3685           
=======================================
  Hits         2610     2610           
  Misses       1075     1075           

Continue to review full report in Codecov by Sentry.

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

@hassila
Copy link
Contributor

hassila commented Nov 14, 2023

LGTM, thanks!

@hassila hassila merged commit d632894 into ordo-one:main Nov 14, 2023
10 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants