Skip to content

feat: add option to hide comments when no changes detected #492

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

Open
rvbsalgado wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat: add option to hide comments when no changes detected #492

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
885ebc6
feat: add option to hide comments when no changes detected
rvbsalgado Mar 6, 2026
7c0dfb4
feat: add option to hide comments when no changes detected
rvbsalgado Mar 6, 2026
568422d
Merge branch 'feat/466-hide-no-changes-comments' of https://github.co…
rvbsalgado Mar 6, 2026
d1e10d6
Merge branch 'feat/466-hide-no-changes-comments' of https://github.co…
rvbsalgado Mar 6, 2026
5609172
Merge branch 'feat/466-hide-no-changes-comments' of https://github.co…
rvbsalgado Mar 9, 2026
7b6d3a8
Merge branch 'feat/466-hide-no-changes-comments' of https://github.co…
rvbsalgado Mar 9, 2026
e351cc6
Merge branch 'feat/466-hide-no-changes-comments' of https://github.co…
rvbsalgado Mar 16, 2026
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
3 changes: 3 additions & 0 deletions cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ func init() {
newStringOpts().
withChoices("hide", "delete").
withDefault("hide"))
boolFlag(flags, "show-no-changes-comment", "Controls whether to post a comment when no changes are detected.",
newBoolOpts().
withDefault(true))
stringSliceFlag(flags, "schemas-location", "Sets schema locations to be used for every check request. Can be a common path on the host or git urls in either git or http(s) format.")
boolFlag(flags, "enable-conftest", "Set to true to enable conftest policy checking of manifests.")
stringSliceFlag(flags, "policies-location", "Sets rego policy locations to be used for every check request. Can be common path inside the repos being checked or git urls in either git or http(s) format.",
Expand Down
1 change: 1 addition & 0 deletions docs/usage.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ The full list of supported environment variables is described below:
|`KUBECHECKS_REPO_REFRESH_INTERVAL`|Interval between static repo refreshes (for schemas and policies).|`5m`|
|`KUBECHECKS_SCHEMAS_LOCATION`|Sets schema locations to be used for every check request. Can be a common path on the host or git urls in either git or http(s) format.|`[]`|
|`KUBECHECKS_SHOW_DEBUG_INFO`|Set to true to print debug info to the footer of MR comments.|`false`|
|`KUBECHECKS_SHOW_NO_CHANGES_COMMENT`|Controls whether to post a comment when no changes are detected.|`true`|
|`KUBECHECKS_TIDY_OUTDATED_COMMENTS_MODE`|Sets the mode to use when tidying outdated comments. One of hide, delete.|`hide`|
|`KUBECHECKS_VCS_BASE_URL`|VCS base url, useful if self hosting gitlab, enterprise github, etc.||
|`KUBECHECKS_VCS_EMAIL`|VCS Email.||
Expand Down
1 change: 1 addition & 0 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ type ServerConfig struct {
ArchiveCacheTTL time.Duration `mapstructure:"archive-cache-ttl"`
SchemasLocations []string `mapstructure:"schemas-location"`
ShowDebugInfo bool `mapstructure:"show-debug-info"`
ShowNoChangesComment bool `mapstructure:"show-no-changes-comment"`
Comment thread
rvbsalgado marked this conversation as resolved.
TidyOutdatedCommentsMode string `mapstructure:"tidy-outdated-comments-mode"`
MaxQueueSize int64 `mapstructure:"max-queue-size"`
MaxConcurrentChecks int `mapstructure:"max-concurrent-checks"`
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ func TestNew(t *testing.T) {
v.Set("worst-conftest-state", "warning")
v.Set("repo-refresh-interval", "10m")
v.Set("additional-apps-namespaces", "default,kube-system")
v.Set("show-no-changes-comment", true)

cfg, err := NewWithViper(v)
require.NoError(t, err)
Expand All @@ -29,4 +30,5 @@ func TestNew(t *testing.T) {
assert.Equal(t, pkg.StateWarning, cfg.WorstConfTestState, "worst states can be overridden")
assert.Equal(t, time.Minute*10, cfg.RepoRefreshInterval)
assert.Equal(t, []string{"default", "kube-system"}, cfg.AdditionalAppsNamespaces)
assert.True(t, cfg.ShowNoChangesComment)
}
6 changes: 4 additions & 2 deletions pkg/events/check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,10 @@ func (ce *CheckEvent) Process(ctx context.Context) error {

if len(ce.affectedItems.Applications) <= 0 && len(ce.affectedItems.ApplicationSets) <= 0 {
ce.logger.Info().Msg("No affected apps or appsets, skipping")
if _, err := ce.ctr.VcsClient.PostMessage(ctx, ce.pullRequest, fmt.Sprintf("## Kubechecks %s Report\nNo changes", ce.ctr.Config.Identifier)); err != nil {
return errors.Wrap(err, "failed to post changes")
if ce.ctr.Config.ShowNoChangesComment {
if _, err := ce.ctr.VcsClient.PostMessage(ctx, ce.pullRequest, fmt.Sprintf("## Kubechecks %s Report\nNo changes", ce.ctr.Config.Identifier)); err != nil {
return errors.Wrap(err, "failed to post no-changes report")
}

@Greyeye Greyeye Mar 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you add debug log that post has been skipped.

Suggested change
}
} else {
ce.logger.Debug().Msg("Skipping no-changes comment (show-no-changes-comment=false)")
}

@Greyeye Greyeye Mar 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@rvbsalgado thank you for the PR, other than the debug print, it looks good on me. we can deploy it on the next release (prob 3.0.0-rc2)

@rvbsalgado rvbsalgado Mar 16, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@Greyeye added your suggestion 🙏

normally when is the next release scheduled?

@Greyeye Greyeye Mar 26, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@rvbsalgado We got no schedule, but happy to tag new version when ready.

I also had another look.
As far as I can see this change will just skip posting back when there is no change. If I guessed it right, we're going to end up a comment like "kubechecks is running" but never gets updated

func (ce *CheckEvent) createNote(ctx context.Context) (*msg.Message, error) {
	ctx, span := otel.Tracer("check").Start(ctx, "createNote")
	defer span.End()

	ce.logger.Info().Msgf("Creating note")

	return ce.ctr.VcsClient.PostMessage(ctx, ce.pullRequest, fmt.Sprintf("## Kubechecks %s Report\n:hourglass: kubechecks running...", ce.ctr.Config.Identifier))
}

kubechecks/pkg/events/check.go

Line 503 in 15ba686

return ce.ctr.VcsClient.PostMessage(ctx, ce.pullRequest, fmt.Sprintf("## Kubechecks %s Report\n:hourglass: kubechecks running...", ce.ctr.Config.Identifier))

@Greyeye Greyeye Mar 26, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can you also add Caller() to the debug?
like
log.Debug().Caller()

}

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

In the no-affected-apps/appsets early-return path, kubechecks skips setting a commit status. With show-no-changes-comment=false this means the run can produce no PR-visible signal at all (no comment and no status), which is likely to confuse users and can leave required status checks in an indeterminate state. Consider setting an explicit success/skip status in this branch (e.g., pkg.StateSkip) regardless of whether the comment is posted.

Suggested change
}
}
// Even when there are no affected apps/appsets, set an explicit skip status
if err := ce.ctr.VcsClient.SetStatus(ctx, ce.pullRequest, pkg.StateSkip, "No affected apps or appsets"); err != nil {
ce.logger.Error().Err(err).Msg("Failed to set skip status for no-op run")
}

Copilot uses AI. Check for mistakes.
Comment on lines +304 to 310

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

The new ShowNoChangesComment behavior in Process is not covered by tests. Since pkg/events/check_test.go already exists for this file, please add a unit test that verifies VcsClient.PostMessage is called when ShowNoChangesComment=true and is not called when it is false (and that the method still returns nil).

Copilot uses AI. Check for mistakes.
return nil
}
Expand Down
Loading