search

18F / concourse-compliance-testing

Concourse CI assets for Compliance Toolkit
https://compliance-viewer.18f.gov/
Other
17 stars 7 forks source link

Slack on change only #55

Closed ctro closed 8 years ago

ctro commented 8 years ago

OK, I did a thing.

I wanted to see how much more readable the summary code would be if I class-ified it. Turns out it's way more readable :) Need to stop watching Rich Hickey talks.

Then I made Slack notify on change only, which was as simple as NOT writing a summary.txt file if there was no change. This is not (yet?) configurable at all.

THEN I ran into the "NO CHANGE" bug and realized it was related to OpenOpps not having any zap issues. So I changed the way that is handled. I'm still not sure if OpenOpps really does have 0 zap issues, or if zap is zapping out.

Tons of RED/GREEN here :( But not really huge changes.

Relates to https://trello.com/c/UnfzMVR9/111-a-user-should-be-able-to-configure-slack-notification-presence-frequency-and-options and https://trello.com/c/y63dRvhw/133-investigate-c2-scans-always-showing-as-new-results

Posting for feedback now, I can get back to this next week. Also, could merge Aidan's S3 thing first and I could deal with whatever merge needs to be done here.

ctro commented 8 years ago

If this is too much I can split the commits out. Mostly wanted to get this out there before the weekend started.

afeld commented 8 years ago

I wanted to see how much more readable the summary code would be if I class-ified it. Turns out it's way more readable :) Need to stop watching Rich Hickey talks.

:laughing:

Then I made Slack notify on change only...This is not (yet?) configurable at all.

That seems like a reasonable default.

I'm still not sure if OpenOpps really does have 0 zap issues

Maybe try running the scan locally through desktop ZAP?

If this is too much I can split the commits out.

If it's easy to do so, that might help keep the PR comments focused, but not a huge deal otherwise.