Conversation
|
@simon3z @pweil- @ilackarms please take a look |
|
@enoodle looks good to me |
|
cc @aweiteka |
| type OpenSCAPMetadata struct { | ||
| Status OpenSCAPStatus // Status of the OpenSCAP scan report | ||
| ErrorMessage string // Error message from the openscap | ||
| ContentTimeStamp string // Timestamp for this data |
There was a problem hiding this comment.
Why is this a string? Would there be a benefit to leaving it as Time? If it is because of issues with json marshalling should we take the Kube approach and provide a wrapper (see unversioned.Time in Kube) so we retain the option to perform time based functions on this field?
There was a problem hiding this comment.
TBH it is string because it was string before [1], I didn't go into this that much when I moved these pieces of code around. I agree that it will be nice to have it as Time but I think that changing this should be done in a different PR because the issue is different than the one this PR is about.
[1]https://github.com/openshift/image-inspector/pull/39/files#diff-f5d24f0dc00fe1c3eeda8d22b2e523ebL20
There was a problem hiding this comment.
ah, ok. Been a while, sorry. I agree that we should leave it for this refactor
|
|
||
| for k, v := range tests { | ||
| ii := &defaultImageInspector{*v.opts, InspectorMetadata{}} | ||
| ii := &defaultImageInspector{*v.opts, iiapi.InspectorMetadata{}, &MockImageServer{}} |
There was a problem hiding this comment.
can't you just pass nil here, why the mock? I guess that means the next question is should we test the nil and non nil use case? 😄
There was a problem hiding this comment.
The tests pass with nil, in [1] we check to see if the image server is defined. I will remove this redundant code for the mock server. It is easy to make again but I don't think it will be needed here anyway.
[1]https://github.com/openshift/image-inspector/pull/39/files#diff-cb14c05f76ccda787aced81d2a624e69R138
4 participants
resubmitting of simon3z#11