Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Allow for multiple reporting formats #39

Merged
merged 8 commits into from
Apr 27, 2023
Merged

feat: Allow for multiple reporting formats #39

merged 8 commits into from
Apr 27, 2023

Conversation

tarkatronic
Copy link
Contributor

Fixes #5

  • All reporting related stuff is now in the reporting package
    • This may require some more thought on organization. I was pretty heavy-handed here.
  • There's now a Reporter interface
  • There's a SlackReporter implementation of that interface
  • Slack functionality is all isolated inside the Slack module
  • Slack messages are sent asynchronously!

Oh also there's no tests here. Which sucks. But this PR is already so massive...

Largely, this PR is shuffling a whole lot of code around to new places.

@tarkatronic
feat(wip): First pass at a reporter interface
f518be6
@tarkatronic
feat!: Major rewrite of reporting functionality
bd4310b 
* All reporting related stuff is now in the `reporting` package
  * This may require some more thought on organization. I was pretty
    heavy-handed here.
* There's now a `Reporter` interface
* There's a `SlackReporter` implementation of that interface
* Slack functionality is all isolated inside the Slack module
* Slack messages are sent asynchronously!

Oh also there's no tests here. Which sucks. But this PR is already so
massive...
@tarkatronic tarkatronic requested review from JoseAngel1196 and a team April 25, 2023 22:50
@codecov
Copy link

codecov bot commented Apr 25, 2023 •
edited
Loading

Codecov Report

Merging #39 (d9f2c25) into main (22da1f4) will increase coverage by 27.28%.
The diff coverage is 56.22%.

@@             Coverage Diff             @@
##             main      #39       +/-   ##
===========================================
+ Coverage   16.71%   44.00%   +27.28%     
===========================================
  Files           9       12        +3     
  Lines         347      400       +53     
===========================================
+ Hits           58      176      +118     
+ Misses        288      223       -65     
  Partials        1        1
Flag Coverage Δ
unittests 44.00% <56.22%> (+27.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
internal/scan.go 0.00% <0.00%> (ø)
reporting/utils.go 0.00% <0.00%> (ø)
reporting/constants.go 100.00% <100.00%> (ø)
reporting/reports.go 100.00% <100.00%> (ø)
reporting/slack.go 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@phylum-io
Copy link

phylum-io bot commented Apr 26, 2023

Phylum OSS Supply Chain Risk Analysis - INCOMPLETE

The analysis contains 1 package(s) Phylum has not yet processed,
preventing a complete risk analysis. Phylum is processing these
packages currently and should complete soon.
Please wait for up to 30 minutes, then re-run the analysis.

View this project in the Phylum UI

@tarkatronic
Copy link
Contributor Author

I apologize for the size of this PR. There was a lot of necessary reshuffling to make this all work right. And once I got started on tests, there was no stopping me. 😅

JoseAngel1196
JoseAngel1196 approved these changes Apr 26, 2023
View reviewed changes
Copy link
Contributor

@JoseAngel1196 JoseAngel1196 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! This is becoming more flexible to work on. Now, I want to get some work done here. 😄

reporting/slack.go
@@ -0,0 +1,175 @@
package reporting
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤩

@tarkatronic tarkatronic requested a review from a team April 26, 2023 20:52
@tarkatronic
Copy link
Contributor Author

Amazing! This is becoming more flexible to work on. Now, I want to get some work done here. 😄

Thanks! I really hope this works out as well as we think it will.

I think I'm going to leave this PR open at least for the night, to give others a chance to pick it apart. As excited as I am to get this merged 😄

@tarkatronic tarkatronic merged commit 0018381 into main Apr 27, 2023
@tarkatronic tarkatronic deleted the feat/multi-reporter branch April 27, 2023 14:32
@tarkatronic tarkatronic mentioned this pull request Apr 27, 2023
Closed
@tarkatronic tarkatronic added this to the Version 1.0 milestone May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoseAngel1196 JoseAngel1196 JoseAngel1196 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Version 1.0
Development

Successfully merging this pull request may close these issues.

Allow for multiple reporting formats
2 participants
@tarkatronic @JoseAngel1196