Skip to content

feat: Add noMerge config option with auto-detection for GitHub Actions #739

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

Merged
BYK merged 1 commit into from
Feb 4, 2026
+427 −55
Merged

feat: Add noMerge config option with auto-detection for GitHub Actions #739

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
42 changes: 32 additions & 10 deletions src/__tests__/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ describe('validateConfiguration', () => {
test('parses configuration with targets', () => {
const data = {
github: { owner: 'getsentry', repo: 'craft' },
targets: [
{ name: 'npm' },
{ name: 'github', tagPrefix: 'v' },
],
targets: [{ name: 'npm' }, { name: 'github', tagPrefix: 'v' }],
};

expect(validateConfiguration(data)).toEqual(data);
Expand Down Expand Up @@ -61,18 +58,21 @@ describe('validateConfiguration', () => {
});

test('fails with invalid github config', () => {
expect(() => validateConfiguration({ github: { owner: 'getsentry' } }))
.toThrow(/repo.*Required/);
expect(() =>
validateConfiguration({ github: { owner: 'getsentry' } }),
).toThrow(/repo.*Required/);
});

test('fails with invalid minVersion format', () => {
expect(() => validateConfiguration({ minVersion: 'invalid' }))
.toThrow(/minVersion/);
expect(() => validateConfiguration({ minVersion: 'invalid' })).toThrow(
/minVersion/,
);
});

test('fails with invalid changelog policy', () => {
expect(() => validateConfiguration({ changelog: { policy: 'invalid' } }))
.toThrow(/changelog/);
expect(() =>
validateConfiguration({ changelog: { policy: 'invalid' } }),
).toThrow(/changelog/);
});
});

Expand All @@ -96,3 +96,25 @@ describe('CraftProjectConfigSchema', () => {
expect(result.success).toBe(false);
});
});

describe('noMerge config', () => {
test('parses configuration with noMerge: true', () => {
const data = { noMerge: true };
expect(validateConfiguration(data)).toEqual(data);
});

test('parses configuration with noMerge: false', () => {
const data = { noMerge: false };
expect(validateConfiguration(data)).toEqual(data);
});

test('noMerge defaults to undefined when not specified', () => {
const data = { github: { owner: 'getsentry', repo: 'craft' } };
const result = validateConfiguration(data);
expect(result.noMerge).toBeUndefined();
});

test('fails with invalid noMerge type', () => {
expect(() => validateConfiguration({ noMerge: 'yes' })).toThrow(/noMerge/);
});
});
71 changes: 44 additions & 27 deletions src/commands/publish.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
DEFAULT_RELEASE_BRANCH_NAME,
getGlobalGitHubConfig,
expandWorkspaceTargets,
getNoMergeConfig,
} from '../config';
import { formatTable, logger } from '../logger';
import { TargetConfig } from '../schemas/project_config';
Expand Down Expand Up @@ -180,10 +181,10 @@ function checkVersion(argv: Arguments<any>, _opt: any): any {
async function publishToTarget(
target: BaseTarget,
version: string,
revision: string
revision: string,
): Promise<void> {
const publishMessage = `=== Publishing to target: ${chalk.bold.cyan(
target.id
target.id,
)} ===`;
const delim = Array(stringLength(publishMessage) + 1).join('=');
logger.info(' ');
Expand All @@ -203,7 +204,7 @@ async function publishToTarget(
version,
revision,
},
}
},
)();
}

Expand All @@ -215,7 +216,7 @@ async function publishToTarget(
*/
async function printRevisionSummary(
artifactProvider: BaseArtifactProvider,
revision: string
revision: string,
): Promise<void> {
const artifacts = await artifactProvider.listArtifactsForRevision(revision);
if (artifacts.length > 0) {
Expand All @@ -236,7 +237,7 @@ async function printRevisionSummary(
head: ['File Name', 'Size', 'Updated', 'ContentType'],
style: { head: ['cyan'] },
},
artifactData
artifactData,
);
logger.info(' ');
logger.info(`Available artifacts: \n${table.toString()}\n`);
Expand All @@ -247,7 +248,7 @@ async function printRevisionSummary(

async function getTargetList(
targetConfigList: TargetConfig[],
artifactProvider: BaseArtifactProvider
artifactProvider: BaseArtifactProvider,
): Promise<BaseTarget[]> {
logger.trace('Initializing targets');
const githubRepo = await getGlobalGitHubConfig();
Expand All @@ -265,7 +266,7 @@ async function getTargetList(
const target = new targetClass(
targetConfig,
artifactProvider,
githubRepo
githubRepo,
);
targetList.push(target);
} catch (err) {
Expand Down Expand Up @@ -293,7 +294,7 @@ async function getTargetList(
async function checkRequiredArtifacts(
artifactProvider: BaseArtifactProvider,
revision: string,
requiredNames?: string[]
requiredNames?: string[],
): Promise<void> {
if (!requiredNames || requiredNames.length === 0) {
return;
Expand All @@ -307,16 +308,16 @@ async function checkRequiredArtifacts(
for (const requiredNameRegexString of requiredNames) {
const requiredNameRegex = stringToRegexp(requiredNameRegexString);
const matchedArtifacts = artifacts.filter(artifact =>
requiredNameRegex.test(artifact.filename)
requiredNameRegex.test(artifact.filename),
);
if (matchedArtifacts.length === 0) {
checkPassed = false;
reportError(
`No matching artifact found for the required pattern: ${requiredNameRegexString}`
`No matching artifact found for the required pattern: ${requiredNameRegexString}`,
);
} else {
logger.debug(
`Artifact "${matchedArtifacts[0].filename}" matches pattern ${requiredNameRegexString}`
`Artifact "${matchedArtifacts[0].filename}" matches pattern ${requiredNameRegexString}`,
);
}
}
Expand All @@ -339,7 +340,7 @@ async function checkRequiredArtifacts(
async function checkRevisionStatus(
statusProvider: BaseStatusProvider,
revision: string,
skipStatusCheckFlag = false
skipStatusCheckFlag = false,
): Promise<void> {
if (skipStatusCheckFlag) {
logger.warn(`Skipping build status checks for revision ${revision}`);
Expand All @@ -354,7 +355,7 @@ async function checkRevisionStatus(
logger.trace(repositoryInfo);
} catch (e) {
logger.error(
`Cannot get repository information from ${statusProvider.config.name}. Check your configuration and credentials.`
`Cannot get repository information from ${statusProvider.config.name}. Check your configuration and credentials.`,
);
reportError(e);
}
Expand All @@ -379,7 +380,7 @@ async function handleReleaseBranch(
remoteName: string,
branch: string,
mergeTarget?: string,
keepBranch = false
keepBranch = false,
): Promise<void> {
if (!mergeTarget) {
mergeTarget = await getDefaultBranch(git, remoteName);
Expand Down Expand Up @@ -413,7 +414,7 @@ async function handleReleaseBranch(
*/
export async function runPostReleaseCommand(
newVersion: string,
postReleaseCommand?: string
postReleaseCommand?: string,
): Promise<boolean> {
let sysCommand: shellQuote.ParseEntry;
let args: shellQuote.ParseEntry[];
Expand All @@ -429,7 +430,7 @@ export async function runPostReleaseCommand(
} else {
// Not running post-release command
logger.info(
`Not running the optional post-release command: '${DEFAULT_POST_RELEASE_SCRIPT_PATH}' not found`
`Not running the optional post-release command: '${DEFAULT_POST_RELEASE_SCRIPT_PATH}' not found`,
);
return false;
}
Expand All @@ -442,7 +443,7 @@ export async function runPostReleaseCommand(
PATH: process.env.PATH,
GITHUB_TOKEN: process.env.GITHUB_TOKEN,
...Object.fromEntries(
ALLOWED_ENV_VARS.map(key => [key, process.env[key]])
ALLOWED_ENV_VARS.map(key => [key, process.env[key]]),
),
},
});
Expand Down Expand Up @@ -473,7 +474,7 @@ export async function publishMain(argv: PublishOptions): Promise<any> {
reportError(
'Your repository is in a dirty state. ' +
'Please stash or commit the pending changes.',
logger
logger,
);
}
}
Expand Down Expand Up @@ -514,14 +515,14 @@ export async function publishMain(argv: PublishOptions): Promise<any> {
let targetsToPublish: Set<string> = new Set(
(typeof argv.target === 'string' ? [argv.target] : argv.target) || [
SpecialTarget.All,
]
],
);

// Treat "all"/"none" specially
for (const specialTarget of [SpecialTarget.All, SpecialTarget.None]) {
if (targetsToPublish.size > 1 && targetsToPublish.has(specialTarget)) {
logger.error(
`Target "${specialTarget}" specified together with other targets. Exiting.`
`Target "${specialTarget}" specified together with other targets. Exiting.`,
);
return undefined;
}
Expand All @@ -544,14 +545,14 @@ export async function publishMain(argv: PublishOptions): Promise<any> {

for (const published of Object.keys(publishState.published)) {
logger.info(
`Skipping target ${published} as it is marked as successful in state file.`
`Skipping target ${published} as it is marked as successful in state file.`,
);
targetsToPublish.delete(published);
}

if (!targetsToPublish.has(SpecialTarget.All)) {
targetConfigList = targetConfigList.filter(targetConf =>
targetsToPublish.has(BaseTarget.getId(targetConf))
targetsToPublish.has(BaseTarget.getId(targetConf)),
);
}

Expand Down Expand Up @@ -585,19 +586,35 @@ export async function publishMain(argv: PublishOptions): Promise<any> {
if (argv.keepDownloads) {
logger.info(
'Directory with the downloaded artifacts will not be removed',
`Path: ${downloadDirectory}`
`Path: ${downloadDirectory}`,
);
}
}, !argv.keepDownloads);

logger.info(' ');
}

if (argv.noMerge) {
logger.info('Not merging per user request via no-merge option.');
// Check both CLI flag and config option for noMerge
const noMergeConfig = getNoMergeConfig();
const noMerge = argv.noMerge || noMergeConfig.noMerge;

// Warn if --merge-target was specified but noMerge is true from config
if (noMergeConfig.noMerge && !argv.noMerge && argv.mergeTarget) {
logger.warn(
`The --merge-target option will be ignored because noMerge is enabled via ${noMergeConfig.source === 'auto-detected' ? 'auto-detection (compiled GitHub Action)' : 'config'}.`,
);
}

if (noMerge) {
const source = argv.noMerge
? 'CLI option'
: noMergeConfig.source === 'auto-detected'
? 'auto-detection (compiled GitHub Action with dist/ folder)'
: 'config';
logger.info(`Not merging the release branch (${source}).`);
} else if (!branchName) {
logger.info(
'Not merging because cannot determine a branch name to merge from.'
'Not merging because cannot determine a branch name to merge from.',
);
} else if (
targetsToPublish.has(SpecialTarget.All) ||
Expand All @@ -610,7 +627,7 @@ export async function publishMain(argv: PublishOptions): Promise<any> {
argv.remote,
branchName,
argv.mergeTarget,
argv.keepBranch
argv.keepBranch,
);
// XXX(BYK): intentionally DO NOT await unlinking as we do not want
// to block (both in terms of waiting for IO and the success of the
Expand Down
Loading
Loading