Skip to content
This repository was archived by the owner on Jan 19, 2024. It is now read-only.

Conversation

@josephperrott
Copy link
Member

Previously, rerunning CI with the tooling would simply request a new run of the workflow
on the branch. Instead, we should request a rerun via the API to allow only running
the failing jobs rather than rerunning the successful jobs.

…cleCI

Previously, rerunning CI with the tooling would simply request a new run of the workflow
on the branch.  Instead, we should request a rerun via the API to allow only running
the failing jobs rather than rerunning the successful jobs.
@google-cla google-cla bot added the cla: yes label Dec 7, 2020
Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

I really like being able to proparly run a workflow as a PR (which to my understanding is what this change allows us to do (among other things)). But I think it might also be useful to run the whole workflow (i.e. including the passed jobs) at HEAD (for example, to verify against recently merged commits).

Could we have too labels (one that reruns the whole workflow and one that reruns the failed jobs only)? 🙏

Comment on lines +118 to +123
for (const status of statuses) {
if (status.context.startsWith('ci/circleci:')) {
targetUrl = status.target_url;
break;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Super-nit: This could be simplified as:

Suggested change
for (const status of statuses) {
if (status.context.startsWith('ci/circleci:')) {
targetUrl = status.target_url;
break;
}
}
const targetUrl = statuses.find(x => x.context.startsWith('ci/circleci:'))?.target_url;

/**
* The matcher results of the regex to select the job ID of the job which the status represents.
*/
const jobIdMatcher = targetUrl.match(`https://circleci.com/gh/${owner}/${repo}/(\d+)\?`);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this string is converted to the RegExp you intend it to be converted 😁
What you have now will basically match https://circleci.com/gh/${owner}/${repo}/ followed by zero or more d letters 😁

You were probably going for something like:

Suggested change
const jobIdMatcher = targetUrl.match(`https://circleci.com/gh/${owner}/${repo}/(\d+)\?`);
const jobIdMatcher = targetUrl.match(`https://circleci\\.com/gh/${owner}/${repo}/(\\d+)\\?`);

}

/** The job ID. */
const job = jobIdMatcher[0];
Copy link
Member

Choose a reason for hiding this comment

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

Didn't you mean:

Suggested change
const job = jobIdMatcher[0];
const job = jobIdMatcher[1];

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants