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
ocp4: Use ScanSettingBindings for e2e tests #6297
Conversation
/test e2e-aws-ocp4-cis-node |
Pull-request updated, HEAD is now dd56b9d |
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.
Some nits inline, mostly looks good
err := backoff.RetryNotify(func() error { | ||
return ctx.dynclient.Create(goctx.TODO(), binding) | ||
}, bo, func(err error, d time.Duration) { | ||
fmt.Printf("Couldn't create binding after %s: %s\n", d.String(), err) |
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.
Is %s OK to use for formatting an error? I've usually seen %v being used.
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.
Hmm, git grep across some code shows it is, please disregard.
These are the objects we recommend folks to use instead of raw ComplianceSuites. It also allows us for auto-detection of platform/node checks.
Pull-request updated, HEAD is now 5a26d84 |
/test e2e-aws-rhcos4-e8 |
/test all |
/retest |
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.
/lgtm
These are the objects we recommend folks to use instead of raw
ComplianceSuites. It also allows us for auto-detection of platform/node
checks.