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

[FEATURE] Validate performance results files input to compare package #208

Closed
mdjastrzebski opened this issue Oct 3, 2022 · 8 comments
Closed
Labels
CLI feature good first issue Good for newcomers help wanted Extra attention is needed

Comments

@mdjastrzebski
Copy link
Member

Is your feature request related to a problem? Please describe.

Currently reading of performance file by compare package does not perform any validation, so the CLI could crash with cryptic errors if file is corrupted.

Describe the solution you'd like
Use zod or other hight quality library to parse/validate contents of the results file. This will require defining schemas for header and entry rows and should be configured to work with TypeScript. In case of error the information about the incorrect line should be displayed.

Describe alternatives you've considered
Write validation by hand, but that would be too not worth the tradeoff of additional deps

Additional context
N/A

@mdjastrzebski mdjastrzebski added good first issue Good for newcomers help wanted Extra attention is needed CLI feature labels Oct 3, 2022
@ShaswatPrabhat
Copy link
Contributor

@mdjastrzebski I can give this a shot if still open.

@mdjastrzebski
Copy link
Member Author

@ShaswatPrabhat go ahead 👍

@ShaswatPrabhat
Copy link
Contributor

@mdjastrzebski Please find the PR

Please let me know if this is along the lines of what you had wanted.

Thanks!

@mdjastrzebski
Copy link
Member Author

@ShaswatPrabhat thanks for submitting the PRs. Unfortunately I'll need to pause a couple of days on my open source work as I've got cold-like symptoms. I'll review the PRs as soon as I get better.

@ShaswatPrabhat
Copy link
Contributor

@mdjastrzebski please take care. Hope you get well soon.

@ShaswatPrabhat
Copy link
Contributor

Hi, are there any other maintainers in @mdjastrzebski 's absence who can please help review or merge the given commits.
I was looking for this PR to add to my hacktoberfest credits.

Thanks in advance.

@mdjastrzebski
Copy link
Member Author

@ShaswatPrabhat I am back and will check your PR today. Thank you for both contribution and panience 🙂

Re other contributors: we also got @thymikee but he is usually even more busy than I am.

@mdjastrzebski
Copy link
Member Author

Resolved by #216

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI feature good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants