Re-generate reports at 2021-07-04 #47
Conversation
Pull Request Test Coverage Report for Build 1026324419
💛 - Coveralls |
| generate-testdata: generate-samples | ||
| generate-testdata: | ||
| docker login https://registry.redhat.io | ||
| make generate-samples |
There was a problem hiding this comment.
Was this not working as a prerequisite? Is there something different/better about calling it explicitly? I think calling it this way will launch a 2nd make as a sub-task to run it.
There was a problem hiding this comment.
I add the docker login there to make it easier for others to understand and to force that is a pre-requirement for the makefile target. The go mod will try to log in as well but not the sample. The motivation here is:
- I do not know how it works
- I know that I need to run this target
- Then, it will fail immediately if the person has not the login and will be easier to get this requirement
There was a problem hiding this comment.
Those seem like reasonable justifications to doing it this way. 👍
| command := exec.Command("mkdir", dashboardPath) | ||
| _, err = pkg.RunCommand(command) | ||
| if err != nil { | ||
| log.Warnf("running command :%s", err) |
There was a problem hiding this comment.
Seems like ERROR level not WARN, kinda dead in the water if it can't mkdir.
There was a problem hiding this comment.
We call that in chan and should no fail. If the dir exist then, it does not need to be created.
These silly fixes are for you folks to be able to run the audit as well in a straightforward way. If we remove the dir or raise an error here the reports will not be gen when you call the make testdata-generate for that. Note that it was a comment before.
There was a problem hiding this comment.
ah, okay, got it. sounds reasonable
|
/lgtm |
|
/approve |
Description
Re-generate reports at 2021-07-04