This creates a situation such that if the PR has the requisite amount of LGTMs but has not passed, the Check will remain pending until another person triggers the check_run.requested_action action.

This should in fact be tripped on cron update.

See electron/electron#26913

We need to mock out the appropriate events and test them in a similar was as we currently do trop.

• Add users name to "Declined by" section in check summary
• Add api-review/declined label to issue

cation seems is adding the new-pr label to my unpublished PR drafts. Example: electron/electron#19657

This is slightly problematic to me because I would want the new-pr 24-hour window to be as soon as I publish the actual PR. Ideally, it would instead add the label at that moment.

For example electron/electron#27200, the PR's title was changed from "fix" to "feat", and the "API Review" job did not show.

The flow between trop & cation here seems weird. It all works out in the end though so maybe the add + remove is intentional?

What it says on the tin. This bot will be used for api-review purposes so it should be slightly more generic to make its purpose clearer.

See https://github.com/electron/electron/pull/37535/checks?check_run_id=12028226514

The original review from @~nornagon was of this format:

Link: electron/electron#37535 (review)

If the new-pr label is not present.

LGTM / Request changes should:

• Check if > 2 approved and exactly 0 people requested changes
  • If so, add api-review/approved label
  • If not, ensure api-review/requested label is present

If new-pr is present, ensure api-review/requested is the current label

This API PR doesn't seem to have the API LGTM check attached: electron/electron#35902

Currently:

What would be automatically done:

@codebytere

it looks like it should: https://github.com/electron/cation/blob/main/src/api-review-state.ts#L130

but it doesn't seem to work: electron/electron#35163 (comment)

cc @codebytere

Seen here: electron/electron#26826

The new-pr label was added but the semver label was missing - per https://github.com/electron/cation/blob/master/src/24-hour-rule.ts#L26-L27 we assume this means that it's semver-major and thus the open time should be a week. However, since the 24 hour rule does not currently listen to the pull_request.labeled event there is no way to tell it that it has a different semver label than initially assumed when one is applied. We should listen to it and update accordingly.

Seen electron/electron#27703 - API review labels should not be applied to backports.

What it says on the tin.

We need to add tests for API Review State management

See https://sentry.io/organizations/electronjs/issues/2797204462/?project=5552563&referrer=slack

addOrUpdateCheck() easy enough 😄

It shouldn't do that.

😢

see electron/electron#35710 (review)

but the check shows 0/2 LGTMs.

electron/electron#35699

When a PR to electron/electron is tagged with semver/minor or semver/major, this bot should automatically label that PR with api-review/requested if the author did not previously add the label or does not have permissions to do so.

Refs electron/electron#36606

When a user who has previously given a PR an API LGTM comment or review then chooses to request changes on a PR, the GitHub check should only display the latest salient review, be that the change request, the decline, or the lgtm.

Once the user's concerns have been addressed and the change request is removed, the most recent salient review would once again be the LGTM, and the output update accordingly.

cc @jkleinsc

Every PR to electron/electron should be required to have semver label:

• semver/patch
• semver/minor
• semver/major

This should be enforced via a GitHub Check, which is a required check. If the label is missing, the check will pend indefinitely.

electron/electron#34974

As shown in this screenshot taken on 2021-04-13, the API review bot did not automatically move the status to approved:

This screenshot is from: https://github.com/electron/electron/pull/28518/checks?check_run_id=2282280655

I was able to work around this issue by manually setting the PR to api-review/approved ✅

See electron/electron#39330

• No api-review/* label --> Cancelled check
• api-review/requested --> Pending check
• api-review/waiting-for-response --> Pending check
• api-review/approved --> Green check

Prevent humans from adding api-review/approved manually or removing any api-review/* labels that are not api-review/requested.

If two members of the API Working Group comment "API LGTM", then the bot should switch the existing api-review/* to api-review/approved

See electron/electron#38420

See electron/electron#36934

This PR illustrates the problem: electron/electron#40301

When an API WG member has reviewed a PR and added feedback, I think we're supposed to add the api-review/reviewed label and remove the api-review/requested label. However, when we do that it looks like Cation automatically adds back the api-review/requested label (which triggers API WG notifications about a new PR).

If I'm understanding our API WG labeling system correctly, Cation should only be adding the api-review/requested label if no other api-review label (including api-review/reviewed) has been applied.

The GitHub checks API doesn't work on forks essentially at all, so we should ensure that at minimum the api-review/requested label is appropriately updated after the semver-dependent open time has passed and the API is approved or declined

Since 24-hour-rule.ts only searches open PRs, anything that gets merged within 24 hours never gets the label cleared.

Examples of old PRs that still have the label: https://github.com/electron/electron/pulls?q=is%3Apr+label%3A%22new-pr+%F0%9F%8C%B1%22+created:%3C2020-06-11T20:51:17