-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Add support for specifying jobs to wait for by name #483
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your suggestion and addressing as this PR!
I have a plan to provide the feature with workflow + job
specifier. Could you check the my review?
jobs-to-wait-for: | ||
description: 'Comma-separated list of job names to wait for' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand it, job names might be duplicated if they are defined in another workflow. So I want to specify combined workflow file name
and job name
when providing this feature.
I guess #475 (comment) is a way to get them.
const otherRelatedRuns = checkRunSummaries.flatMap<CheckRunsSummary>((summary) => | ||
const filteredCheckRunSummaries = jobsToWaitFor.length === 0 | ||
? checkRunSummaries | ||
: checkRunSummaries.filter(summary => jobsToWaitFor.includes(summary.source.name)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Array.prototype.includes()
is a loop in a loop. So please prefer Set.prototype.has() as
wait-other-jobs/src/github-api.ts
Line 104 in bc780bf
ownJobIDs.has(summary.source.id) ? [] : [summary] |
ChatGPT answer that addresses your points To implement the feature of waiting for only a subset of jobs, you can follow these steps:
# action.yml
...
inputs:
...
workflow-job-map:
description: 'JSON object that maps workflow names to arrays of job names to wait for'
required: false
default: '{}'
// src/main.ts
...
const workflowJobMapInput = JSON.parse(getInput('workflow-job-map', { required: true, trimWhitespace: true }));
const workflowJobMap = new Map<string, Set<string>>();
for (const workflowName in workflowJobMapInput) {
workflowJobMap.set(workflowName, new Set(workflowJobMapInput[workflowName]));
}
...
export async function fetchOtherRunStatus(
// eslint-disable-next-line @typescript-eslint/prefer-readonly-parameter-types
octokit: Parameters<typeof fetchRunSummaries>[0],
params: Parameters<typeof fetchRunSummaries>[1],
ownJobIDs: Readonly<Set<JobID>>,
workflowJobMap: Map<string, Set<string>>,
): Promise<Report> {
...
}
const checkRunSummaries = await fetchRunSummaries(octokit, params);
const filteredCheckRunSummaries = workflowJobMap.size === 0
? checkRunSummaries
: checkRunSummaries.filter(summary => {
const workflowName = summary.source.name.split('/')[0].trim();
const jobName = summary.source.name.split('/')[1].trim();
return (
workflowJobMap.has(workflowName) &&
workflowJobMap.get(workflowName)!.has(jobName)
);
});
const otherRelatedRuns = filteredCheckRunSummaries.flatMap<CheckRunsSummary>((summary) =>
ownJobIDs.has(summary.source.id) ? [] : [summary]
);
...
// src/main.ts
...
const report = await fetchOtherRunStatus(
octokit,
{ ...repositoryInfo, ref: commitSha },
ownJobIDs,
workflowJobMap,
);
... Now the action will only wait for the specified subset of jobs within the given workflows. If the Users can now specify workflow and job names like this: - uses: kachick/wait-for-other-jobs@v1
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
workflow-job-map: '{"Workflow1": ["Job1", "Job2"], "Workflow2": ["JobA"]}' |
const workflowName = summary.source.name.split('/')[0].trim();
const jobName = summary.source.name.split('/')[1].trim();
This is why I referred to another comment. Using GraphQL API might be a better way to get workflow names. Using only REST APIs requires many send requests. |
This has not been tested, but should achieve the desired behaviour.
Closes #474