-
Notifications
You must be signed in to change notification settings - Fork 1
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
Summary format#89 #103
Summary format#89 #103
Conversation
Conflicts: cmd/Commands.go
@@ -0,0 +1,29 @@ | |||
package core | |||
|
|||
type MinMaxMeanTotalInt struct { |
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.
I would call this type Stat or something. If we add StdDev we then have to change the name of the type.
Random bool | ||
Summary bool | ||
SummaryFormat string `json:"summary-format"` | ||
SummaryBuilder core.SummaryBuilder |
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.
I would say a SummaryBuilder takes a configuration as opposed to a Configuration being coupled to a SummaryBuilder.
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.
corcel-report.html.gz
corcel-report.json
Could you remove these from git? Not sure if they came in on this branch or they are from before but best to get shut - didn't notice until now. Could you add them to the .gitignore too ?
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.
All looks good. If you could remove those files - it looks ace! 👍
easier. Adding test coverage for yaml PlanBuilder
If you look at the files in the PR I am actually already deleting them in this! |
closes #89