feat: Use Slack Block Kit for report formatting #56
Conversation
|
@SteveKekacs I got all the tests fixed up now for this refactor. The one planned piece I haven't done yet is the team report summary line. Specifically the breakdown by severity as seen in your example here How vital do you feel that is to this overall rework? I'm debating leaving that as a future enhancement as well as the sorting order, to not let perfect be the enemy of good. What are your thoughts on that? |
Codecov Report
@@ Coverage Diff @@
## main #56 +/- ##
==========================================
+ Coverage 48.07% 52.01% +3.94%
==========================================
Files 13 13
Lines 441 496 +55
==========================================
+ Hits 212 258 +46
- Misses 228 235 +7
- Partials 1 3 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Yeah totally fine IMO to do as a follow up 👍 |
| } | ||
| reporters = append(reporters, &reporting.ConsoleReporter{Config: userConfig}) | ||
|
|
||
| reportTime := time.Now().Format(time.RFC1123) |
There was a problem hiding this comment.
[q] do we want to explicitly set UTC() here?
Looking at the source code, I don't think it actually changes anything
https://github.com/golang/go/blob/go1.20.4/src/time/time.go#L1129
since setLoc ignores UTC
https://github.com/golang/go/blob/go1.20.4/src/time/time.go#L207-L209
There was a problem hiding this comment.
This is a great call! Especially when we create the Datadog reporter, I think we will definitely want to have this normalized to UTC. And in fact, taking it one step further, I think that we will want to pass just the raw timestamp to reporters and let them format it.
Even one step further beyond that, we can introduce a config option to let users control the formatting of the timestamp. 😄
| return nil | ||
| } | ||
| if teamInfo.Slack_channel == "" { | ||
| log.Debug().Str("team", teamID).Any("repos", repos).Msg("Skipping report since Slack channel is not configured.") |
There was a problem hiding this comment.
[q] This seems like more than a Debug? Either an Info or Warn since important configuration is missing?
There was a problem hiding this comment.
I really debated back and forth on that one. The reason I went with debug is because, with a sufficient size org, this could get pretty spammy. I remember at $previousEmployer, there were thousands of repos, and that would just be... way too much.
Though I suppose at info level it wouldn't show by default 🤔 We may want to just go through and do a pass on log levels in general.
Fixes #46
WIP! The following is a full checklist of items to complete:
Summary line has a breakdown by severity(Moving to future enhancement)This changes from super simple text formatting to using full Slack Block Kit functionality. Open to suggestions on further improvements here!
Notably, one request from #46 that is not solved here is sorting repositories in the team report by level of vuln severity. That felt like a whole other "thing", and easy enough to split out into a separate enhancement. A follow-up feature request will be filed to cover that.