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.

Sign up for GitHub

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

Jump to bottom

feat(cascading): option to bypass non-latest minor branch #2473

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+141 −15
Open

feat(cascading): option to bypass non-latest minor branch #2473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/.cascadingrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "../apps/github-cascading-app/schemas/config.schema.json",
"bypassReviewers": true,
"labels": ["cascading"],
"onlyCascadeOnHighestMinors": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably not activate it for us

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why ? I was thinking that it is actually the issue we have today with the non-removed prerelease branches and the policy supporting only the latest minor of each major?

"ignoredPatterns": [
"-next$"
]
Expand Down
5 changes: 5 additions & 0 deletions apps/github-cascading-app/schemas/config.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
"description": "Pattern determining if the branch is part of the cascading strategy",
"default": "^releases?/\\d+\\.\\d+"
},
"onlyCascadeOnHighestMinors": {
"type": "boolean",
"description": "Determine if the branches for which a higher minor version exists should be skipped during the cascading",
"default": false
},
"versionCapturePattern": {
"type": "string",
"description": "Pattern containing a capture to extract the version of a cascading branch",
Expand Down
87 changes: 87 additions & 0 deletions apps/github-cascading-app/src/cascading/cascading.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,93 @@ describe('Cascading Application', () => {
expect(logger.info).toHaveBeenCalledWith('The branch test-cascading/1.0 is the last branch of the cascading. The process will stop.');
});

describe('on onlyCascadeOnHighestMinors options', () => {

it('should ignore branches not to the latest minor when true', async () => {
customization.loadConfiguration = customization.loadConfiguration.mockResolvedValue({
...DEFAULT_CONFIGURATION,
cascadingBranchesPattern: 'test-cascading/.*',
onlyCascadeOnHighestMinors: true,
ignoredPatterns: []
});
customization.getBranches = customization.getBranches.mockResolvedValue([
'test-cascading/1.0',
'test-cascading/1.1',
'test-cascading/1.2',
'test-cascading/2.0'
]);
customization.isBranchAhead = customization.isBranchAhead.mockResolvedValue(true);
customization.createBranch = customization.createBranch.mockResolvedValue();
customization.getPullRequests = customization.getPullRequests.mockResolvedValue([]);
customization.createPullRequest = customization.createPullRequest.mockResolvedValue({
id: 1,
originBranchName: '',
isOpen: true,
mergeable: true,
body: render(mockBasicTemplate, { isConflicting: false, targetBranch: 'main', currentBranch: 'release/0.1', bypassReviewers: true }, { async: false })
cpaulve-1A marked this conversation as resolved.
Show resolved Hide resolved
});
await expect(customization.cascade('test-cascading/1.0')).resolves.not.toThrow();
expect(logger.info).toHaveBeenCalledWith('Cascading plugin execution');
expect(customization.isBranchAhead).toHaveBeenCalledWith('test-cascading/1.0', 'test-cascading/1.2');
});

it('should ignore branches until latest if needed', async () => {
customization.loadConfiguration = customization.loadConfiguration.mockResolvedValue({
...DEFAULT_CONFIGURATION,
cascadingBranchesPattern: 'test-cascading/.*',
onlyCascadeOnHighestMinors: true,
defaultBranch: 'main',
ignoredPatterns: []
});
customization.getBranches = customization.getBranches.mockResolvedValue([
'test-cascading/1.0',
'main'
]);
customization.isBranchAhead = customization.isBranchAhead.mockResolvedValue(true);
customization.createBranch = customization.createBranch.mockResolvedValue();
customization.getPullRequests = customization.getPullRequests.mockResolvedValue([]);
customization.createPullRequest = customization.createPullRequest.mockResolvedValue({
id: 1,
originBranchName: '',
isOpen: true,
mergeable: true,
body: render(mockBasicTemplate, { isConflicting: false, targetBranch: 'main', currentBranch: 'release/0.1', bypassReviewers: true }, { async: false })
});
await expect(customization.cascade('test-cascading/1.0')).resolves.not.toThrow();
expect(logger.info).toHaveBeenCalledWith('Cascading plugin execution');
expect(customization.isBranchAhead).toHaveBeenCalledWith('test-cascading/1.0', 'main');
});

it('should consider branches not to the latest minor when false', async () => {
customization.loadConfiguration = customization.loadConfiguration.mockResolvedValue({
...DEFAULT_CONFIGURATION,
cascadingBranchesPattern: 'test-cascading/.*',
onlyCascadeOnHighestMinors: false,
ignoredPatterns: []
});
customization.getBranches = customization.getBranches.mockResolvedValue([
'test-cascading/1.0',
'test-cascading/1.1',
'test-cascading/1.2',
'test-cascading/2.0'
]);
customization.isBranchAhead = customization.isBranchAhead.mockResolvedValue(true);
customization.createBranch = customization.createBranch.mockResolvedValue();
customization.getPullRequests = customization.getPullRequests.mockResolvedValue([]);
customization.createPullRequest = customization.createPullRequest.mockResolvedValue({
id: 1,
originBranchName: '',
isOpen: true,
mergeable: true,
body: render(mockBasicTemplate, { isConflicting: false, targetBranch: 'main', currentBranch: 'release/0.1', bypassReviewers: true }, { async: false })
});
await expect(customization.cascade('test-cascading/1.0')).resolves.not.toThrow();
expect(logger.info).toHaveBeenCalledWith('Cascading plugin execution');
expect(customization.isBranchAhead).toHaveBeenCalledWith('test-cascading/1.0', 'test-cascading/1.1');
});

});

it('should skip ignored branch if not ahead', async () => {
customization.loadConfiguration = customization.loadConfiguration.mockResolvedValue({
...DEFAULT_CONFIGURATION,
Expand Down
54 changes: 41 additions & 13 deletions apps/github-cascading-app/src/cascading/cascading.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { coerce, compare, parse, valid } from 'semver';
import { coerce, compare, lte, parse, type SemVer, valid } from 'semver';
import { BaseLogger, CascadingConfiguration, CascadingPullRequestInfo, CheckConclusion, PullRequestContext } from './interfaces';
import { renderFile } from 'ejs';
import { resolve } from 'node:path';
Expand All @@ -15,6 +15,9 @@ export const CASCADING_BRANCH_PREFIX = 'cascading';
/** Time (in ms) to wait before re-checking the mergeable status of a PR */
export const RETRY_MERGEAGLE_STATUS_CHECK_TIMING = 3000;

/** Object representing a branch with the version determined from its name */
type BranchObject = { branch: string; semver: SemVer | undefined };

/**
* Handles the cascading to the next branch
*/
Expand Down Expand Up @@ -171,7 +174,8 @@ export abstract class Cascading {
};
}
})
.filter(({branch, semver}) => {
.filter((branchObject): branchObject is BranchObject => {
const { branch, semver } = branchObject;
if (semver === null) {
this.logger.warn(`Failed to parse the branch ${branch}, it will be skipped from cascading`);
return false;
Expand All @@ -193,7 +197,7 @@ export abstract class Cascading {
}

/**
* Generate teh cascading branch name
* Generate the cascading branch name
* @param baseVersion Version extracted from the base branch
* @param targetVersion Version extracted from the target branch
* @param configurations
Expand Down Expand Up @@ -340,6 +344,36 @@ export abstract class Cascading {
return !checkboxLine?.[0]?.match(/^ *- \[x]/i);
}

protected getTargetBranch(cascadingBranches: BranchObject[], currentBranchName: string, config: CascadingConfiguration) {
const branchIndex = cascadingBranches.findIndex(({ branch }) => branch === currentBranchName);
if (branchIndex < 0) {
this.logger.error(`The branch ${currentBranchName} is not part of the list of cascading branch. The process will stop.`);
return;
}

if (branchIndex === cascadingBranches.length - 1) {
this.logger.info(`The branch ${currentBranchName} is the last branch of the cascading. The process will stop.`);
return;
}

const targetBranchIndex = branchIndex + 1;
if (!config.onlyCascadeOnHighestMinors) {
return cascadingBranches[targetBranchIndex];
}

for (let i = targetBranchIndex; i < cascadingBranches.length; i++) {
const targetBranch = cascadingBranches[i];
const { semver } = targetBranch;
if (!semver) {
return targetBranch;
}

if (cascadingBranches.slice(i + 1).every((otherBranch) => otherBranch.semver?.major !== semver.major || (otherBranch.semver && lte(otherBranch.semver, semver)))) {
return targetBranch;
}
}
}

/**
* Launch the cascading process
* @param currentBranchName name of the branch to cascade (ex: release/8.0)
Expand All @@ -365,20 +399,14 @@ export abstract class Cascading {
this.logger.info('Cascading plugin execution');
const branches = await this.getBranches();
const cascadingBranches = this.getOrderedCascadingBranches(branches, config);
const branchIndex = cascadingBranches.findIndex(({ branch }) => branch === currentBranchName);
const targetBranch = this.getTargetBranch(cascadingBranches, currentBranchName, config);

if (branchIndex < 0) {
this.logger.error(`The branch ${currentBranchName} is not part of the list of cascading branch. The process will stop.`);
return;
}

if (branchIndex === cascadingBranches.length - 1) {
this.logger.info(`The branch ${currentBranchName} is the last branch of the cascading. The process will stop.`);
if (!targetBranch) {
this.logger.info(`No target branch found for the cascading from ${currentBranchName}. The process will stop.`);
return;
}

const currentBranch = cascadingBranches[branchIndex];
const targetBranch = cascadingBranches[branchIndex + 1];
const currentBranch = cascadingBranches.find(({ branch }) => branch === currentBranchName)!;
const cascadingBranch = this.determineCascadingBranchName(currentBranch.semver?.format() || currentBranch.branch, targetBranch.semver?.format() || targetBranch.branch, config);
const isAhead = await this.isBranchAhead(currentBranch.branch, targetBranch.branch);

Expand Down
8 changes: 7 additions & 1 deletion apps/github-cascading-app/src/cascading/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ export interface CascadingConfiguration {
defaultBranch: string;
/** Pattern determining if the branch is part of the cascading strategy */
cascadingBranchesPattern: string;
/** Determine if the branches for which a higher minor version exists should be skipped during the cascading */
onlyCascadeOnHighestMinors: boolean;
/** Pattern containing a capture to extract the version of a cascading branch */
versionCapturePattern: string;
/** Bypass the reviewers validation for the pull request, only the CI checks will be executed */
Expand All @@ -34,7 +36,10 @@ export interface PullRequestContext {
currentBranch: string;
/** Cascading Pull Request Target Branch */
targetBranch: string;
/** Determine if the reviewers are bypassed */
/**
* Determine if the reviewers are bypassed
* Note: This option is not supported on Github anymore due to Github Api change.
Copy link
Contributor

@matthieu-crouzet matthieu-crouzet Nov 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why are not we deprecating it ?
If we want to be able to re-use that part of the code for another platform,
we may extract that part in a dedicated package
else it seems to be stricly done for GitHub and then we should deprecate this proprety

Copy link
Contributor Author

@kpanot kpanot Nov 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I did not deprecate this option because I think that as the cascading is already today platform agnostic (event if it is implemented only for Github) and this option still make sense for supporting platform, it would be weird to remove it in 1 year to follow the unique implementation to potentially seeing it reappear after a while because of a new implementation.
It made more sense for me to keep it and warn the Github users instead, but I am OK to change if needed.

PS: To implement another platform support, there is no need to extract code, it's the bot that should switch the implementation according to the received message. So today it is already ready to support multi platform as it is

**/
bypassReviewers: boolean;
/** Is the an update of the {@link currentBranch} conflicting */
isConflicting: boolean;
Expand Down Expand Up @@ -68,6 +73,7 @@ export const DEFAULT_CONFIGURATION: Readonly<CascadingConfiguration> = {
ignoredPatterns: [] as string[],
defaultBranch: '',
cascadingBranchesPattern: '^releases?/\\d+\\.\\d+',
onlyCascadeOnHighestMinors: false,
versionCapturePattern: '/((?:0|[1-9]\\d*)\\.(?:0|[1-9]\\d*)(?:\\.0-[^ ]+)?)$',
bypassReviewers: false,
labels: [] as string[],
Expand Down
Loading