Allow specification of custom checklists from inside of client repo #20
Conversation
There was a problem hiding this comment.
looks just lovely, thank you! I appreciate the example which reads the file; I think we're underusing that (and over-reporting checklist needs.)
One thing while the boundary is hardening here: do you have any idea how we could examine the diff? For example, we could see if a fuzz test is actually being added before asking someone to check the fuzz testing item off.
Just gonna have to owe you a GIF; I'm on a plane and my connection is really bad right now.
|
Oh interesting, yea we could define some way to either derive from a class that receives all files or one that receives all lines of diff? Or we could change the relevent_for callback to receive an object that contains the full diff and list of files? I think my preference is the 2nd one, and it shouldn't be too hard to extend that to handle that later. That might pair nicely with existing work we did for the complexity report like: https://github.com/NoRedInk/deploy-complexity/blob/master/lib/deploy_complexity/changed_files.rb#L6, or some of the info we collect in https://github.com/NoRedInk/deploy-complexity/blob/master/lib/deploy_complexity/deploy.rb#L66? I'm going to make an issue in this repo linking to this PR with a request for followup so we don't lose this. |
|
Sounds great. Regardless, we control both sides of this boundary so it's fine to merge this PR as-is. 👍
… On Jan 22, 2020, at 1:57 PM, Charles Comstock ***@***.***> wrote:
Oh interesting, yea we could define some way to either derive from a class that receives all files or one that receives all lines of diff? Or we could change the relevent_for callback to receive an object that contains the full diff and list of files? I think my preference is the 2nd one, and it shouldn't be too hard to extend that to handle that later.
That might pair nicely with existing work we did for the complexity report like: https://github.com/NoRedInk/deploy-complexity/blob/master/lib/deploy_complexity/changed_files.rb#L6 <https://github.com/NoRedInk/deploy-complexity/blob/master/lib/deploy_complexity/changed_files.rb#L6>, or some of the info we collect in https://github.com/NoRedInk/deploy-complexity/blob/master/lib/deploy_complexity/deploy.rb#L66 <https://github.com/NoRedInk/deploy-complexity/blob/master/lib/deploy_complexity/deploy.rb#L66>?
I'm going to make an issue in this repo linking to this PR with a request for followup so we don't lose this.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub <#20?email_source=notifications&email_token=AACWYSJD22ISY4QQGHSFMBLQ7CQLNA5CNFSM4KH4BER2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJU4YUY#issuecomment-577358931>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACWYSIHTDEIMSB7PNLPKNLQ7CQLNANCNFSM4KH4BERQ>.
|
Specify custom deploy checklist items from within the monolith. Previously checklists were all defined & used here. There are a few provided as examples now, but moves the bulk of specification local to the repo being instrumented. This prevents accidental leaking of secrets & credentials, and allows checklists to mutate and customize according to the target repo's needs. This does make the API boundary for creating checklists a little firmer though by specifying it in two places.