-
Notifications
You must be signed in to change notification settings - Fork 17
Re-generate reports at 2021-07-04 #47
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,17 +52,11 @@ func main() { | |
pathToWalk := filepath.Join(fullReportsPath, dir) | ||
dashboardPath := filepath.Join(fullReportsPath, dir, "dashboards") | ||
|
||
//command := exec.Command("rm", "-rf", dashboardPath) | ||
//_, err = pkg.RunCommand(command) | ||
//if err != nil { | ||
// log.Errorf("running command :%s", err) | ||
//} | ||
|
||
//command = exec.Command("mkdir", dashboardPath) | ||
//_, err = pkg.RunCommand(command) | ||
//if err != nil { | ||
// log.Errorf("running command :%s", err) | ||
//} | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like ERROR level not WARN, kinda dead in the water if it can't mkdir. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, okay, got it. sounds reasonable |
||
} | ||
|
||
if _, err := os.Stat(pathToWalk); err != nil && os.IsNotExist(err) { | ||
continue | ||
|
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.
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.
Uh oh!
There was an error while loading. Please reload this page.
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 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:
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.
Those seem like reasonable justifications to doing it this way. 👍