Skip to content

Commit

Permalink
renamed all cases of min_approvals to minApprovals
Browse files Browse the repository at this point in the history
  • Loading branch information
Bullrich committed Sep 21, 2023
1 parent bb670b9 commit 258a746
Show file tree
Hide file tree
Showing 16 changed files with 160 additions and 160 deletions.
14 changes: 7 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ rules:
exclude:
- '.github/'
type: basic
min_approvals: 2
minApprovals: 2
teams:
- team-1
- team-2
Expand All @@ -251,7 +251,7 @@ It has the same parameters as a normal rule:
- **exclude**: An array of regular expressions pointing to the files that this rule should ignore when deciding if it should evaluate the Pull Request.
- **optional**
- **type**: This must be the string `basic`.
- **min_approvals**: The number of approvals that are need to fulfill this condition.
- **minApprovals**: The number of approvals that are need to fulfill this condition.
- It can not be lower than 1.
- **Optional**: Defaults to 1.
- Must be greater than the number of users available (you cannot request 5 approvals from a team of 4 users)
Expand Down Expand Up @@ -286,7 +286,7 @@ rules:
- user-2
- teams:
- team-abc
min_approvals: 2
minApprovals: 2
```
- The **name** and **conditions** fields have the same requirements that the `basic` rule has.
- **type**: Must be `or`, `and` or `and-distinct`.
Expand All @@ -298,7 +298,7 @@ rules:
- Must have at least two options.
- If you only need 1 option, then use the `basic` rule.
- Each of the options have the following fields:
- **min_approvals**: The number of approvals that are need to fulfill this condition.
- **minApprovals**: The number of approvals that are need to fulfill this condition.
- It can not be lower than 1.
- **Optional**: Defaults to 1.
- Must be greater than the number of users available (you cannot request 5 approvals from a team of 4 users)
Expand All @@ -319,7 +319,7 @@ reviewers:
- user-2
- teams:
- team-abc
min_approvals: 2
minApprovals: 2
```
This rule will be approved with *any of the following* conditions:
- If a user who belongs to `team-example` or is `user-1` or `user-2` approves the PR.
Expand All @@ -339,7 +339,7 @@ reviewers:
- user-2
- teams:
- team-abc
min_approvals: 2
minApprovals: 2
```
This rule will be approved if *all the following conditions get fulfilled*:
- *One* user who belongs to `team-example` or is `user-1` or `user-2` approves the PR.
Expand Down Expand Up @@ -367,7 +367,7 @@ The fellows rule has a slight difference to all of the rules:
- 'example'
type: fellows
minRank: 2
min_approvals: 2
minApprovals: 2
```
The biggest difference is that it doesn’t have a reviewers type (it doesn’t have a `teams` or `users` field); instead, it has a `minRank` field.

Expand Down
6 changes: 3 additions & 3 deletions src/rules/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ export enum RuleTypes {
Fellows = "fellows",
}

export type Reviewers = { users?: string[]; teams?: string[]; min_approvals: number };
export type Reviewers = { users?: string[]; teams?: string[]; minApprovals: number };

export interface Rule {
name: string;
condition: { include: string[]; exclude?: string[] };
allowedToSkipRule?: Omit<Reviewers, "min_approvals">;
allowedToSkipRule?: Omit<Reviewers, "minApprovals">;
countAuthor?: boolean;
}

Expand All @@ -37,7 +37,7 @@ export interface AndDistinctRule extends Rule {
export interface FellowsRule extends Rule {
type: RuleTypes.Fellows;
minRank: number;
min_approvals: number;
minApprovals: number;
}

export interface ConfigurationFile {
Expand Down
8 changes: 4 additions & 4 deletions src/rules/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const reviewersObj = {
teams: Joi.array().items(Joi.string()).optional().empty(null),
};

const reviewerConditionObj = { ...reviewersObj, min_approvals: Joi.number().min(1).default(1) };
const reviewerConditionObj = { ...reviewersObj, minApprovals: Joi.number().min(1).default(1) };

/** Base rule condition.
* This are the minimum requirements that all the rules must have.
Expand All @@ -24,7 +24,7 @@ const ruleSchema = Joi.object<Rule & { type: string }>().keys({
include: Joi.array().items(Joi.string()).required(),
exclude: Joi.array().items(Joi.string()).optional().allow(null),
}),
allowedToSkipRule: Joi.object<Omit<Reviewers, "min_approvals">>().keys(reviewersObj).optional().or("users", "teams"),
allowedToSkipRule: Joi.object<Omit<Reviewers, "minApprovals">>().keys(reviewersObj).optional().or("users", "teams"),
type: Joi.string()
.valid(RuleTypes.Basic, RuleTypes.And, RuleTypes.Or, RuleTypes.AndDistinct, RuleTypes.Fellows)
.required(),
Expand All @@ -40,7 +40,7 @@ export const generalSchema = Joi.object<ConfigurationFile>().keys({
});

/** Basic rule schema
* This rule is quite simple as it only has the min_approvals field and the required reviewers
* This rule is quite simple as it only has the minApprovals field and the required reviewers
*/
export const basicRuleSchema = Joi.object<BasicRule>()
.keys({ ...reviewerConditionObj, countAuthor: Joi.boolean().default(false) })
Expand All @@ -58,7 +58,7 @@ export const otherRulesSchema = Joi.object<AndRule>().keys({
export const fellowsRuleSchema = Joi.object<FellowsRule>().keys({
countAuthor: Joi.boolean().default(false),
minRank: Joi.number().required().min(1).empty(null),
min_approvals: Joi.number().min(1).default(1),
minApprovals: Joi.number().min(1).default(1),
});

/**
Expand Down
20 changes: 10 additions & 10 deletions src/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,11 @@ export class ActionRunner {
// We get all the users belonging to each 'and distinct' review condition
for (const reviewers of rule.reviewers) {
const users = await this.fetchAllUsers(reviewers);
requirements.push({ users, requiredApprovals: reviewers.min_approvals });
requirements.push({ users, requiredApprovals: reviewers.minApprovals });
}

// We count how many reviews are needed in total
const requiredAmountOfReviews = rule.reviewers.map((r) => r.min_approvals).reduce((a, b) => a + b, 0);
const requiredAmountOfReviews = rule.reviewers.map((r) => r.minApprovals).reduce((a, b) => a + b, 0);
// We get the list of users that approved the PR
const approvals = await this.prApi.listApprovedReviewsAuthors(rule.countAuthor ?? false);

Expand Down Expand Up @@ -393,7 +393,7 @@ export class ActionRunner {
* @see-also ReviewError
*/
async evaluateCondition(
rule: { min_approvals: number } & Reviewers,
rule: { minApprovals: number } & Reviewers,
countAuthor: boolean = false,
): Promise<ReviewState> {
this.logger.debug(JSON.stringify(rule));
Expand All @@ -405,9 +405,9 @@ export class ActionRunner {
throw new Error("No users have been found in the required reviewers");
}

if (requiredUsers.length < rule.min_approvals) {
if (requiredUsers.length < rule.minApprovals) {
this.logger.error(
`${rule.min_approvals} approvals are required but only ${requiredUsers.length} user's approval count.`,
`${rule.minApprovals} approvals are required but only ${requiredUsers.length} user's approval count.`,
);
if (rule.teams) {
this.logger.error(`Allowed teams: ${JSON.stringify(rule.teams)}`);
Expand All @@ -423,7 +423,7 @@ export class ActionRunner {
this.logger.info(`Found ${approvals.length} approvals.`);

// This is the amount of reviews required. To succeed this should be 0 or lower
let missingReviews = rule.min_approvals;
let missingReviews = rule.minApprovals;
for (const requiredUser of requiredUsers) {
// We check for the approvals, if it is a required reviewer we lower the amount of missing reviews
if (approvals.indexOf(requiredUser) > -1) {
Expand Down Expand Up @@ -462,9 +462,9 @@ export class ActionRunner {
throw new Error(`No users have been found with the rank ${rule.minRank} or above`);
}

if (requiredUsers.length < rule.min_approvals) {
if (requiredUsers.length < rule.minApprovals) {
this.logger.error(
`${rule.min_approvals} approvals are required but only ${requiredUsers.length} user's approval count.`,
`${rule.minApprovals} approvals are required but only ${requiredUsers.length} user's approval count.`,
);
throw new Error("The amount of required approvals is smaller than the amount of available users.");
}
Expand All @@ -474,7 +474,7 @@ export class ActionRunner {
this.logger.info(`Found ${approvals.length} approvals.`);

// This is the amount of reviews required. To succeed this should be 0 or lower
let missingReviews = rule.min_approvals;
let missingReviews = rule.minApprovals;
for (const requiredUser of requiredUsers) {
// We check for the approvals, if it is a required reviewer we lower the amount of missing reviews
if (approvals.indexOf(requiredUser) > -1) {
Expand Down Expand Up @@ -531,7 +531,7 @@ export class ActionRunner {
* @param reviewers Object with users or teams to fetch members
* @returns an array with all the users
*/
async fetchAllUsers(reviewers: Omit<Reviewers, "min_approvals">): Promise<string[]> {
async fetchAllUsers(reviewers: Omit<Reviewers, "minApprovals">): Promise<string[]> {
const users: Set<string> = new Set<string>();
if (reviewers.teams) {
for (const team of reviewers.teams) {
Expand Down
16 changes: 8 additions & 8 deletions src/test/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ rules:
- ^\.cargo/.*
exclude:
- ^./gitlab/pipeline/zombienet.yml$
min_approvals: 2
minApprovals: 2
type: basic
teams:
- ci
Expand All @@ -27,7 +27,7 @@ rules:
exclude:
- ^polkadot/runtime\/(kusama|polkadot)\/src\/weights\/.+\.rs$
- ^substrate\/frame\/.+\.md$
min_approvals: 2
minApprovals: 2
allowedToSkipRule:
teams:
- core-devs
Expand All @@ -51,7 +51,7 @@ rules:
- ^\.gitlab-ci\.yml
- ^(?!.*\.dic$|.*spellcheck\.toml$)scripts/ci/.*
- ^\.github/.*
min_approvals: 2
minApprovals: 2
type: basic
teams:
- core-devs
Expand All @@ -67,10 +67,10 @@ rules:
- ^cumulus/parachains/common/src/[^/]+\.rs$
type: and-distinct
reviewers:
- min_approvals: 1
- minApprovals: 1
teams:
- locks-review
- min_approvals: 1
- minApprovals: 1
teams:
- polkadot-review

Expand All @@ -80,7 +80,7 @@ rules:
condition:
include:
- ^cumulus/bridges/.*
min_approvals: 1
minApprovals: 1
teams:
- bridges-core

Expand All @@ -92,10 +92,10 @@ rules:
- ^substrate/frame/(?!.*(nfts/.*|uniques/.*|babe/.*|grandpa/.*|beefy|merkle-mountain-range/.*|contracts/.*|election|nomination-pools/.*|staking/.*|aura/.*))
type: "and"
reviewers:
- min_approvals: 2
- minApprovals: 2
teams:
- core-devs
- min_approvals: 1
- minApprovals: 1
teams:
- frame-coders

Expand Down
20 changes: 10 additions & 10 deletions src/test/rules/andDistinct.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ describe("'AndDistinctRule' rule parsing", () => {
reviewers:
- teams:
- team-example
- min_approvals: 2
- minApprovals: 2
users:
- abc
teams:
Expand All @@ -125,13 +125,13 @@ describe("'AndDistinctRule' rule parsing", () => {
expect(rule.reviewers).toHaveLength(2);
expect(rule.reviewers[0].teams).toContainEqual("team-example");
expect(rule.reviewers[0].users).toBeUndefined();
expect(rule.reviewers[1].min_approvals).toEqual(2);
expect(rule.reviewers[1].minApprovals).toEqual(2);
expect(rule.reviewers[1].users).toContainEqual("abc");
expect(rule.reviewers[1].teams).toContainEqual("xyz");
});
});

test("should default min_approvals to 1", async () => {
test("should default minApprovals to 1", async () => {
api.getConfigFile.mockResolvedValue(`
rules:
- name: Test review
Expand All @@ -150,13 +150,13 @@ describe("'AndDistinctRule' rule parsing", () => {
const config = await runner.getConfigFile("");
const [rule] = config.rules;
if (rule.type === "and-distinct") {
expect(rule.reviewers[0].min_approvals).toEqual(1);
expect(rule.reviewers[0].minApprovals).toEqual(1);
} else {
throw new Error(`Rule type ${rule.type} is invalid`);
}
});

test("should fail with min_approvals in negative", async () => {
test("should fail with minApprovals in negative", async () => {
api.getConfigFile.mockResolvedValue(`
rules:
- name: Test review
Expand All @@ -167,16 +167,16 @@ describe("'AndDistinctRule' rule parsing", () => {
- 'example'
type: and-distinct
reviewers:
- min_approvals: -99
- minApprovals: -99
users:
- user-example
`);
await expect(runner.getConfigFile("")).rejects.toThrowError(
'"reviewers[0].min_approvals" must be greater than or equal to 1',
'"reviewers[0].minApprovals" must be greater than or equal to 1',
);
});

test("should fail with min_approvals in 0", async () => {
test("should fail with minApprovals in 0", async () => {
api.getConfigFile.mockResolvedValue(`
rules:
- name: Test review
Expand All @@ -187,12 +187,12 @@ describe("'AndDistinctRule' rule parsing", () => {
- 'example'
type: and-distinct
reviewers:
- min_approvals: 0
- minApprovals: 0
users:
- user-example
`);
await expect(runner.getConfigFile("")).rejects.toThrowError(
'"reviewers[0].min_approvals" must be greater than or equal to 1',
'"reviewers[0].minApprovals" must be greater than or equal to 1',
);
});
});
Loading

0 comments on commit 258a746

Please sign in to comment.