-
Notifications
You must be signed in to change notification settings - Fork 5
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
Skip passed results for both report table and fetching logs #49
Conversation
Works as intended, thanks @pholica!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from my point of view, thanks!
Please just append the CHANGELOG, README and add code comments, we don't need to bulk up my technical debt, thank you!
Also the debug messages would be nice as well :)
Thank you for the review. I'm going to fix as much as it's reasonable before I leave for one week. It's possible that not all of the items will be addressed this (and effectively next week). |
Those debug messages also serve as comments describing what's happening in the code.
Hmmm, the readme update touches also other options as I merely ran the |
@danmyway Please review, I believe it should be now eligible for merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and the changes are super helpful!
Thanks, @pholica
Resolves #42