Skip to content
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

Refactor of failed reviews objects #112

Merged
merged 14 commits into from
Jan 25, 2024
2 changes: 1 addition & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ outputs:

runs:
using: 'docker'
image: 'docker://ghcr.io/paritytech/review-bot/action:2.3.1'
image: 'Dockerfile'
48 changes: 48 additions & 0 deletions src/failures/commonRules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { summary } from "@actions/core";

import { toHandle } from "../util";
import { RequiredReviewersData, ReviewFailure, RuleFailedSummary } from "./types";

export class CommonRuleFailure extends ReviewFailure {
public readonly usersToRequest: string[];
public readonly teamsToRequest: string[];

constructor(report: RuleFailedSummary & RequiredReviewersData) {
super(report);
this.usersToRequest = report.usersToRequest ?? [];
this.teamsToRequest = report.teamsToRequest ?? [];
}

generateSummary(): typeof summary {
let text = super.generateSummary();

if (this.usersToRequest.length > 0) {
text = text.addHeading("Missing users", 3).addList(this.usersToRequest);
}
if (this.teamsToRequest.length > 0) {
text = text.addHeading("Missing reviews from teams", 3).addList(this.teamsToRequest);
}

if (this.countingReviews.length > 0) {
text = text
.addHeading("Users approvals that counted towards this rule", 3)
.addEOL()
.addList(this.countingReviews.map(toHandle))
.addEOL();
}

if (this.missingUsers.length > 0)
text = text.addDetails(
"GitHub users whose approval counts",
`This is a list of all the GitHub users whose approval would count towards fulfilling this rule:\n\n - ${this.missingUsers
.map(toHandle)
.join("\n - ")}`,
);

return text;
}

getRequestLogins(): { users: string[]; teams: string[] } {
return { users: this.usersToRequest, teams: this.teamsToRequest };
}
}
44 changes: 44 additions & 0 deletions src/failures/fellowsRules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { summary } from "@actions/core";

import { toHandle } from "../util";
import { ReviewFailure, RuleFailedSummary } from "./types";

export class FellowMissingRankFailure extends ReviewFailure {
constructor(
ruleInfo: RuleFailedSummary,
public readonly missingRank: number,
) {
super(ruleInfo);
}

generateSummary(): typeof summary {
let text = super.generateSummary();

text = text
.addHeading("Missing reviews from Fellows", 3)
.addEOL()
.addRaw(`Missing reviews from rank \`${this.missingRank}\` or above`)
.addEOL();
if (this.missingUsers.length > 0)
text = text.addDetails(
"GitHub users whose approval counts",
`This is a list of all the GitHub users who are rank ${this.missingRank} or above:\n\n - ${this.missingUsers
.map(toHandle)
.join("\n - ")}`,
);

if (this.countingReviews.length > 0) {
text = text
.addHeading("Users approvals that counted towards this rule", 3)
.addEOL()
.addList(this.countingReviews.map(toHandle))
.addEOL();
}

return text;
}

getRequestLogins(): { users: string[]; teams: string[] } {
return { users: [], teams: [] };
}
}
3 changes: 3 additions & 0 deletions src/failures/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export * from "./types";
export * from "./commonRules";
export * from "./fellowsRules";
86 changes: 86 additions & 0 deletions src/failures/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { summary } from "@actions/core";

import { RuleTypes } from "../rules/types";

/** Object containing the report on why a rule failed */
export type RuleFailedReport = {
/** The amount of missing reviews to fulfill the requirements */
missingReviews: number;
/** The users who would qualify to complete those reviews */
missingUsers: string[];
/** If applicable, reviews that count towards this rule */
countingReviews: string[];
};

export type RuleFailedSummary = {
type: RuleTypes;
name: string;
} & RuleFailedReport;

export type RequiredReviewersData = {
/** If applicable, the teams that should be requested to review */
teamsToRequest?: string[];
/** If applicable, the users that should be requested to review */
usersToRequest?: string[];
};

/** Class which contains the reports of a failed rule
* Here you can find details on why a rule failed and what requirements it has
*/
export abstract class ReviewFailure {
public readonly name: string;
public readonly type: RuleTypes;
/** The amount of missing reviews */
public readonly missingReviews: number;

/** Approvals that counted towards this rule */
public readonly countingReviews: string[];

/** List of users who would classify to approve this rule */
public readonly missingUsers: string[];

constructor(ruleInfo: RuleFailedSummary) {
this.name = ruleInfo.name;
this.type = ruleInfo.type;
this.missingReviews = ruleInfo.missingReviews;
this.countingReviews = ruleInfo.countingReviews;
this.missingUsers = ruleInfo.missingUsers;
}

ruleExplanation(type: RuleTypes): string {
switch (type) {
case RuleTypes.Basic:
return "Rule 'Basic' requires a given amount of reviews from users/teams";
case RuleTypes.And:
return "Rule 'And' has many required reviewers/teams and requires all of them to be fulfilled.";
case RuleTypes.Or:
return "Rule 'Or' has many required reviewers/teams and requires at least one of them to be fulfilled.";
case RuleTypes.AndDistinct:
return (
"Rule 'And Distinct' has many required reviewers/teams and requires all of them to be fulfilled **by different users**.\n\n" +
"The approval of one user that belongs to _two teams_ will count only towards one team."
);
case RuleTypes.Fellows:
return "Rule 'Fellows' requires a given amount of reviews from users whose Fellowship ranking is the required rank or great.";
default:
console.error("Out of range for rule type", type);
throw new Error("Unhandled rule");
}
}

generateSummary(): typeof summary {
return summary
.emptyBuffer()
.addHeading(this.name, 2)
.addHeading(`Missing ${this.missingReviews} review${this.missingReviews > 1 ? "s" : ""}`, 4)
.addDetails(
"Rule explanation",
this.ruleExplanation(this.type) +
"\n\n" +
"For more info found out how the rules work in [Review-bot types](https://github.com/paritytech/review-bot#types)",
);
}

/** Get the users/teams whose review should be requested */
abstract getRequestLogins(): { users: string[]; teams: string[] };
}
11 changes: 8 additions & 3 deletions src/github/check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export class GitHubChecksApi {
this.logger.debug(`Found match: ${JSON.stringify(check)}`);
await this.api.rest.checks.update({ ...checkData, check_run_id: check.id });
this.logger.debug("Updated check data");
await this.writeSummary(checkData, check.html_url ?? "");
return;
}
}
Expand All @@ -57,16 +58,20 @@ export class GitHubChecksApi {

const check = await this.api.rest.checks.create(checkData);

await this.writeSummary(checkData, check.data.html_url ?? "");

this.logger.debug(JSON.stringify(check.data));
}

private async writeSummary(checkResult: CheckData, resultUrl: string) {
// We publish it in the action summary
await summary
.emptyBuffer()
.addHeading(checkResult.output.title)
// We redirect to the check as it can changed if it is triggered again
.addLink("Find the result here", check.data.html_url ?? "")
.addLink("Find the result here", resultUrl)
.addBreak()
.addRaw(checkResult.output.text)
.write();

this.logger.debug(JSON.stringify(check.data));
}
}
Loading
Loading