Skip to content

Commit 2d28d72

Browse files
leetroutclaude
andcommitted
refactor: hide no-changes comment instead of skipping; rename input
Per reviewer feedback on PR #414: - Post the comment but minimize it via GitHub GraphQL when there are no changes, preserving the PR_RELEASE_MARKER for idempotency checks - Rename input from disable-no-changes-comment to hide-no-changes-pr-comment and camelCase field from disableNoChangesComment to hideNoChangesPrComment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 3e702b0 commit 2d28d72

14 files changed

Lines changed: 126 additions & 53 deletions

File tree

__tests__/config.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ describe('config', () => {
338338
expect(config.disableWiki).toBe(false);
339339
expect(config.wikiSidebarChangelogMax).toBe(5);
340340
expect(config.disableBranding).toBe(false);
341-
expect(config.disableNoChangesComment).toBe(false);
341+
expect(config.hideNoChangesPrComment).toBe(false);
342342
expect(config.githubToken).toBe('ghp_test_token_2c6912E7710c838347Ae178B4');
343343
expect(config.modulePathIgnore).toEqual([]);
344344
expect(config.moduleChangeExcludePatterns).toEqual(['.gitignore', '*.md', '*.tftest.hcl', 'tests/**']);
@@ -368,7 +368,7 @@ describe('config', () => {
368368
['Use Version Prefix: true'],
369369
['Module Ref Mode: tag'],
370370
['Pre-release: false'],
371-
['Disable No-Changes Comment: false'],
371+
['Hide No-Changes PR Comment: false'],
372372
]);
373373
});
374374
});

__tests__/helpers/octokit.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,7 @@ export function createDefaultOctokitMock(): OctokitRestApi {
342342
})();
343343
},
344344
},
345+
graphql: vi.fn().mockResolvedValue({ minimizeComment: { minimizedComment: { isMinimized: true } } }),
345346
};
346347

347348
// For now, we just emulate and return a Partial OctokitRestApi since this is a mocked version

__tests__/main.test.ts

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ describe('main', () => {
6363
context.isPrMergeEvent = false;
6464
config.disableWiki = false;
6565
config.deleteLegacyTags = true;
66-
config.disableNoChangesComment = false;
66+
config.hideNoChangesPrComment = false;
6767

6868
// Reset mocks with default values
6969
vi.mocked(hasReleaseComment).mockResolvedValue(false);
@@ -229,14 +229,15 @@ describe('main', () => {
229229
vi.mocked(TerraformModule.getTagsToDelete).mockReturnValue([]);
230230
});
231231

232-
it('should skip PR comment when no version changes and disable-no-changes-comment is true', async () => {
233-
config.disableNoChangesComment = true;
232+
it('should call addReleasePlanComment (for hiding) when no version changes and hide-no-changes-pr-comment is true', async () => {
233+
config.hideNoChangesPrComment = true;
234234
vi.mocked(TerraformModule.getModulesNeedingRelease).mockReturnValue([]);
235+
vi.mocked(getWikiStatus).mockReturnValue({ status: WIKI_STATUS.SUCCESS });
235236

236237
await run();
237238

238-
expect(addReleasePlanComment).not.toHaveBeenCalled();
239-
expect(info).toHaveBeenCalledWith('No version changes detected. Skipping PR comment.');
239+
// addReleasePlanComment is always called; minimization happens inside it
240+
expect(addReleasePlanComment).toHaveBeenCalled();
240241

241242
// Should NOT call merge-specific functions
242243
expect(createTaggedReleases).not.toHaveBeenCalled();
@@ -246,17 +247,17 @@ describe('main', () => {
246247
expect(setOutput).toHaveBeenCalled();
247248
});
248249

249-
it('should skip PR comment when no module changes and delete-legacy-tags is disabled even with items to delete', async () => {
250-
config.disableNoChangesComment = true;
250+
it('should call addReleasePlanComment when no module changes and delete-legacy-tags is disabled even with items to delete', async () => {
251+
config.hideNoChangesPrComment = true;
251252
config.deleteLegacyTags = false;
252253
vi.mocked(TerraformModule.getModulesNeedingRelease).mockReturnValue([]);
253254
vi.spyOn(TerraformModule, 'getReleasesToDelete').mockReturnValue([{ id: 1, title: 'old', body: '', tagName: 'modules/old/v0' }]);
254255
vi.spyOn(TerraformModule, 'getTagsToDelete').mockReturnValue(['modules/old/v0']);
256+
vi.mocked(getWikiStatus).mockReturnValue({ status: WIKI_STATUS.SUCCESS });
255257

256258
await run();
257259

258-
expect(addReleasePlanComment).not.toHaveBeenCalled();
259-
expect(info).toHaveBeenCalledWith('No version changes detected. Skipping PR comment.');
260+
expect(addReleasePlanComment).toHaveBeenCalled();
260261
});
261262

262263
it('should handle non-merge event (pull request event) when modules need release', async () => {
@@ -307,18 +308,18 @@ describe('main', () => {
307308
});
308309
});
309310

310-
it('should skip PR comment when disable-no-changes-comment is true and no changes', async () => {
311-
config.disableNoChangesComment = true;
311+
it('should call addReleasePlanComment when hide-no-changes-pr-comment is true and no changes', async () => {
312+
config.hideNoChangesPrComment = true;
312313
vi.mocked(TerraformModule.getModulesNeedingRelease).mockReturnValue([]);
314+
vi.mocked(getWikiStatus).mockReturnValue({ status: WIKI_STATUS.SUCCESS });
313315

314316
await run();
315317

316-
expect(addReleasePlanComment).not.toHaveBeenCalled();
317-
expect(info).toHaveBeenCalledWith('No version changes detected. Skipping PR comment.');
318+
expect(addReleasePlanComment).toHaveBeenCalled();
318319
});
319320

320-
it('should still post PR comment when disable-no-changes-comment is true but modules need release', async () => {
321-
config.disableNoChangesComment = true;
321+
it('should post PR comment visibly when hide-no-changes-pr-comment is true but modules need release', async () => {
322+
config.hideNoChangesPrComment = true;
322323
vi.mocked(TerraformModule.getModulesNeedingRelease).mockReturnValue([mockTerraformModuleNeedingRelease]);
323324
vi.mocked(getWikiStatus).mockReturnValue({ status: WIKI_STATUS.SUCCESS });
324325

@@ -327,8 +328,8 @@ describe('main', () => {
327328
expect(addReleasePlanComment).toHaveBeenCalled();
328329
});
329330

330-
it('should still post PR comment when disable-no-changes-comment is true but there are legacy items to delete', async () => {
331-
config.disableNoChangesComment = true;
331+
it('should post PR comment visibly when hide-no-changes-pr-comment is true but there are legacy items to delete', async () => {
332+
config.hideNoChangesPrComment = true;
332333
vi.mocked(TerraformModule.getModulesNeedingRelease).mockReturnValue([]);
333334
vi.mocked(TerraformModule.getReleasesToDelete).mockReturnValue([{ id: 99, title: 'old', body: '', tagName: 'old/v1.0.0' }]);
334335
vi.mocked(getWikiStatus).mockReturnValue({ status: WIKI_STATUS.SUCCESS });

__tests__/pull-request.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,61 @@ describe('pull-request', () => {
473473
);
474474
});
475475

476+
it('should minimize comment when hide-no-changes-pr-comment is true and no changes', async () => {
477+
const nodeId = 'IC_test_node_id';
478+
config.set({ hideNoChangesPrComment: true, deleteLegacyTags: true });
479+
stubOctokitReturnData('issues.createComment', {
480+
data: { id: 1, node_id: nodeId, html_url: 'https://github.com/org/repo/pull/1#issuecomment-1' },
481+
});
482+
stubOctokitReturnData('issues.listComments', { data: [] });
483+
484+
await addReleasePlanComment([], [], [], { status: WIKI_STATUS.SUCCESS });
485+
486+
expect(context.octokit.graphql).toHaveBeenCalledWith(
487+
expect.stringContaining('minimizeComment'),
488+
{ id: nodeId },
489+
);
490+
expect(info).toHaveBeenCalledWith(expect.stringContaining('Minimized comment'));
491+
});
492+
493+
it('should not minimize comment when hide-no-changes-pr-comment is true but modules need release', async () => {
494+
config.set({ hideNoChangesPrComment: true });
495+
stubOctokitReturnData('issues.createComment', {
496+
data: { id: 1, node_id: 'IC_node', html_url: 'https://github.com/org/repo/pull/1#issuecomment-1' },
497+
});
498+
stubOctokitReturnData('issues.listComments', { data: [] });
499+
500+
await addReleasePlanComment(terraformModules, [], [], { status: WIKI_STATUS.SUCCESS });
501+
502+
expect(context.octokit.graphql).not.toHaveBeenCalled();
503+
});
504+
505+
it('should not minimize comment when hide-no-changes-pr-comment is true but there are releases to delete', async () => {
506+
config.set({ hideNoChangesPrComment: true, deleteLegacyTags: true });
507+
stubOctokitReturnData('issues.createComment', {
508+
data: { id: 1, node_id: 'IC_node', html_url: 'https://github.com/org/repo/pull/1#issuecomment-1' },
509+
});
510+
stubOctokitReturnData('issues.listComments', { data: [] });
511+
512+
await addReleasePlanComment([], [{ id: 99, title: 'old', body: '', tagName: 'old/v1.0.0' }], [], {
513+
status: WIKI_STATUS.SUCCESS,
514+
});
515+
516+
expect(context.octokit.graphql).not.toHaveBeenCalled();
517+
});
518+
519+
it('should not minimize comment when hide-no-changes-pr-comment is false', async () => {
520+
config.set({ hideNoChangesPrComment: false });
521+
stubOctokitReturnData('issues.createComment', {
522+
data: { id: 1, node_id: 'IC_node', html_url: 'https://github.com/org/repo/pull/1#issuecomment-1' },
523+
});
524+
stubOctokitReturnData('issues.listComments', { data: [] });
525+
526+
await addReleasePlanComment([], [], [], { status: WIKI_STATUS.SUCCESS });
527+
528+
expect(context.octokit.graphql).not.toHaveBeenCalled();
529+
});
530+
476531
it('should include modules to remove when flag enabled', async () => {
477532
const modulesToRemove = ['legacy-module1', 'legacy-module2'];
478533

__tests__/utils/metadata.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ describe('utils/metadata', () => {
1919
'wiki-sidebar-changelog-max',
2020
'wiki-usage-template',
2121
'disable-branding',
22-
'disable-no-changes-comment',
22+
'hide-no-changes-pr-comment',
2323
'module-path-ignore',
2424
'module-change-exclude-patterns',
2525
'module-asset-exclude-patterns',
@@ -61,7 +61,7 @@ describe('utils/metadata', () => {
6161
'delete-legacy-tags',
6262
'disable-wiki',
6363
'disable-branding',
64-
'disable-no-changes-comment',
64+
'hide-no-changes-pr-comment',
6565
'use-ssh-source-format',
6666
'use-version-prefix',
6767
'pre-release',
@@ -134,7 +134,7 @@ describe('utils/metadata', () => {
134134
'wiki-sidebar-changelog-max': 'wikiSidebarChangelogMax',
135135
'wiki-usage-template': 'wikiUsageTemplate',
136136
'disable-branding': 'disableBranding',
137-
'disable-no-changes-comment': 'disableNoChangesComment',
137+
'hide-no-changes-pr-comment': 'hideNoChangesPrComment',
138138
'module-path-ignore': 'modulePathIgnore',
139139
'module-change-exclude-patterns': 'moduleChangeExcludePatterns',
140140
'module-asset-exclude-patterns': 'moduleAssetExcludePatterns',

action.yml

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,13 @@ inputs:
111111
or where third-party branding is undesirable.
112112
required: true
113113
default: "false"
114-
disable-no-changes-comment:
114+
hide-no-changes-pr-comment:
115115
description: >
116-
When set to true, suppresses the PR comment when there are no module changes and no legacy
117-
tags/releases to delete. Has no effect when there are changes to report; comments are always
118-
posted in that case. By default, this is false so a comment is always posted.
116+
When set to true, hides (minimizes) the PR comment when there are no module changes and no
117+
legacy tags/releases to delete. The comment is still posted so the release marker remains
118+
present for idempotency checks, but it will appear collapsed/hidden by default. Has no effect
119+
when there are changes to report; comments are always posted visibly in that case. By default,
120+
this is false so a comment is always posted visibly.
119121
required: true
120122
default: "false"
121123
module-path-ignore:

dist/index.js

Lines changed: 18 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/index.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ function initializeConfig(): Config {
127127
info(`Use Version Prefix: ${configInstance.useVersionPrefix}`);
128128
info(`Module Ref Mode: ${configInstance.moduleRefMode}`);
129129
info(`Pre-release: ${configInstance.preRelease}`);
130-
info(`Disable No-Changes Comment: ${configInstance.disableNoChangesComment}`);
130+
info(`Hide No-Changes PR Comment: ${configInstance.hideNoChangesPrComment}`);
131131

132132
return configInstance;
133133
} finally {

src/main.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,10 @@ function initialize(): { config: Config; context: Context } {
3333
* @returns {Promise<void>} Resolves when wiki-related operations are completed.
3434
*/
3535
async function handlePullRequestEvent(
36-
config: Config,
3736
terraformModules: TerraformModule[],
3837
releasesToDelete: GitHubRelease[],
3938
tagsToDelete: string[],
4039
): Promise<void> {
41-
const modulesNeedingRelease = TerraformModule.getModulesNeedingRelease(terraformModules);
42-
const hasDeleteWork = config.deleteLegacyTags && (releasesToDelete.length > 0 || tagsToDelete.length > 0);
43-
44-
if (config.disableNoChangesComment && modulesNeedingRelease.length === 0 && !hasDeleteWork) {
45-
info('No version changes detected. Skipping PR comment.');
46-
return;
47-
}
48-
4940
const wikiStatusResult = getWikiStatus();
5041
await addReleasePlanComment(terraformModules, releasesToDelete, tagsToDelete, wikiStatusResult);
5142

@@ -228,7 +219,7 @@ export async function run(): Promise<void> {
228219
if (context.isPrMergeEvent) {
229220
await handlePullRequestMergedEvent(config, terraformModules, releasesToDelete, tagsToDelete);
230221
} else {
231-
await handlePullRequestEvent(config, terraformModules, releasesToDelete, tagsToDelete);
222+
await handlePullRequestEvent(terraformModules, releasesToDelete, tagsToDelete);
232223
}
233224
} catch (error) {
234225
if (error instanceof Error) {

0 commit comments

Comments
 (0)