Implement core functionality #11
Conversation
| const eventLink = getEventLink() | ||
|
|
||
| const mrkdwn = [ | ||
| status ? emojiFromStatus(status) : emoji('black_square_button'), |
There was a problem hiding this comment.
I don't personally love the initial / in progress / "unchecked" icon:
Perhaps we make this configurable at some point.
There was a problem hiding this comment.
That'd be sweet! Big fan of using custom emojis :D
There was a problem hiding this comment.
Tracking via #13.
src/github/stage.ts
Outdated
| const slackRegex = /[^A-Za-z]slack[^A-Za-z]/i | ||
| const lastCompletedSlackStep = data.jobs | ||
| .flatMap<CompletedJobStep>(({steps}) => { | ||
| if (!steps) { | ||
| return [] | ||
| } | ||
|
|
||
| return steps | ||
| .filter(isCompletedJobStep) | ||
| .filter(({name}) => slackRegex.test(` ${name} `)) | ||
| }) | ||
| .pop() |
There was a problem hiding this comment.
This won't work well with parallel jobs. Something to probably revisit in #12.
There was a problem hiding this comment.
- So this grab the last completed slack step, regardless of what job it is?
- Do you think taking in the job name as an optional parameter would solve for this?
If one is yes, I'm actually a bit surprised this works. I guess the assumption here is the jobs array returned is "ordered". I wonder however, if I have a job in the middle of my workflow but it runs last, would that break this?
There was a problem hiding this comment.
So this grab the last completed slack step, regardless of what job it is?
Correct.
In a sequential workflow (in which jobs depend on each other), this seems to work fine; the listJobsForWorkflowRun response does not return jobs that have not yet started.
Do you think taking in the job name as an optional parameter would solve for this?
Do you mean an optional input specifying the previous / dependent job name? We inherently know which job we are currently in, but we actually need to know which previous job / step to compute duration.
There was a problem hiding this comment.
Having given this some more thought, I am leaning towards scoping "stage" duration to its contextual job. We'll still want to support multiple Slack steps per job, but I now see less value in computing this across jobs, especially given the threaded message displays the job's name.
There was a problem hiding this comment.
Updated in c6eb5f9.
It still favors a lastCompletedSlackStep but scoped within the current job, which still allows for scenarios in which multiple slack notifications can be sent within the same job, i.e.
steps:
- name: Post to Slack
- name: Deploy to staging
- name: Post to Slack
- name: Deploy to prod
- name: Post to Slack
src/github/stage.ts
Outdated
| const slackRegex = /[^A-Za-z]slack[^A-Za-z]/i | ||
| const lastCompletedSlackStep = data.jobs | ||
| .flatMap<CompletedJobStep>(({steps}) => { | ||
| if (!steps) { | ||
| return [] | ||
| } | ||
|
|
||
| return steps | ||
| .filter(isCompletedJobStep) | ||
| .filter(({name}) => slackRegex.test(` ${name} `)) | ||
| }) | ||
| .pop() |
There was a problem hiding this comment.
- So this grab the last completed slack step, regardless of what job it is?
- Do you think taking in the job name as an optional parameter would solve for this?
If one is yes, I'm actually a bit surprised this works. I guess the assumption here is the jobs array returned is "ordered". I wonder however, if I have a job in the middle of my workflow but it runs last, would that break this?
| const eventLink = getEventLink() | ||
|
|
||
| const mrkdwn = [ | ||
| status ? emojiFromStatus(status) : emoji('black_square_button'), |
There was a problem hiding this comment.
That'd be sweet! Big fan of using custom emojis :D
@devreferences