Add an API endpoint to generate a CSV compliance report #167
Conversation
yiraeChristineKim
force-pushed
the
ACM-6884
branch
2 times, most recently
from
825fdd8 to
e69ffa9
Compare
yiraeChristineKim
force-pushed
the
ACM-6884
branch
from
c1ba460 to
a34432e
Compare
yiraeChristineKim
force-pushed
the
ACM-6884
branch
from
a34432e to
0e7e43e
Compare
yiraeChristineKim
force-pushed
the
ACM-6884
branch
2 times, most recently
from
1455258 to
a1b3dc5
Compare
|
@JustinKuli @mprahl Ready again Thanks!!! |
yiraeChristineKim
force-pushed
the
ACM-6884
branch
from
a1b3dc5 to
52cff6b
Compare
yiraeChristineKim
force-pushed
the
ACM-6884
branch
from
52cff6b to
f42aa4d
Compare
| return vv | ||
| case int32: | ||
| // All int32 related id | ||
| if int(vv) == 0 { |
There was a problem hiding this comment.
0 mean id is empty
|
@mprahl @JustinKuli Ready Again! Thanks all |
| case int32: | ||
| // All int32 related id | ||
| if int(vv) == 0 { | ||
| return "nil" |
There was a problem hiding this comment.
I think no value is more readable in a spreadsheet.
| return "nil" | |
| return "" |
There was a problem hiding this comment.
How about
case JSONMap:
if vv == nil {
return "nil"
}
Since most nil cases change to ""? should we change this too?
There was a problem hiding this comment.
jsonByte, err := json.MarshalIndent(vv, "", " ") output like this:
"{
""test"": true
}"
There was a problem hiding this comment.
But in Mac spreadsheet and slack spreadsheet looks fine
{
"severity": "low",
"test": "one"
}
| return vv.String() | ||
| case pq.StringArray: | ||
| // nil will be [] | ||
| return fmt.Sprintf("%v", vv) |
There was a problem hiding this comment.
Could you do a strings.Join on this so that you get val1, val2. This is more friendly than an array like syntax.
There was a problem hiding this comment.
Join with a comma is confusing because CSV is comma-based. Is there any other way??
There was a problem hiding this comment.
Does the CSV writer not escape the commas for you?
There was a problem hiding this comment.
strings.join output is like this:
"cat-3, cat-4"
which looks good to me
| return fmt.Sprintf("%v", vv) | ||
| default: | ||
| // case nil: | ||
| return "nil" |
There was a problem hiding this comment.
Shouldn't this be a programming error if we encounter a type we do not know how to convert?
There was a problem hiding this comment.
I think at least, you could return this:
| return "nil" | |
| return fmt.Sprintf("%v", vv) |
There was a problem hiding this comment.
Error looks like this:
,configmaps [common] found in namespace default,
Ref: https://issues.redhat.com/browse/ACM-6884 Signed-off-by: Yi Rae Kim <[email protected]>
yiraeChristineKim
force-pushed
the
ACM-6884
branch
from
f42aa4d to
ef8c2a0
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mprahl, yiraeChristineKim The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Generating a report as described in the design.
Ref: https://issues.redhat.com/browse/ACM-6884