feat: add option to hide comments when no changes detected#492
feat: add option to hide comments when no changes detected#492rvbsalgado wants to merge 7 commits into
Conversation
Adds KUBECHECKS_SHOW_NO_CHANGES_COMMENT env var to control whether kubechecks posts a comment when no affected apps are found. Closes issue zapier#466 Signed-off-by: rvbsalgado <rvbsalgado@users.noreply.github.com>
Adds KUBECHECKS_SHOW_NO_CHANGES_COMMENT env var to control whether kubechecks posts a comment when no affected apps are found. Closes issue zapier#466 Signed-off-by: rvbsalgado <rvbsalgado@users.noreply.github.com>
…m/rvbsalgado/kubechecks into feat/466-hide-no-changes-comments Signed-off-by: rvbsalgado <rvbsalgado@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a configurable option to suppress “No changes” PR/MR comments when kubechecks determines no Applications/ApplicationSets are affected, reducing comment noise in multi-cluster/monorepo setups (Issue #466).
Changes:
- Gate posting the “No changes” report comment behind a new
show-no-changes-commentconfig flag. - Add
ShowNoChangesCommentto server configuration and expose it as a CLI flag / env var (KUBECHECKS_SHOW_NO_CHANGES_COMMENT). - Document the new environment variable in
docs/usage.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/events/check.go | Conditionally posts (or skips) the “No changes” comment based on config. |
| pkg/config/config.go | Adds ShowNoChangesComment to the config struct for Viper unmarshal. |
| cmd/root.go | Introduces the --show-no-changes-comment boolean flag with default true. |
| docs/usage.md | Documents KUBECHECKS_SHOW_NO_CHANGES_COMMENT and its default. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…m/rvbsalgado/kubechecks into feat/466-hide-no-changes-comments Signed-off-by: rvbsalgado <rvbsalgado@users.noreply.github.com>
…m/rvbsalgado/kubechecks into feat/466-hide-no-changes-comments
| 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") | ||
| } |
There was a problem hiding this comment.
can you add debug log that post has been skipped.
| } | |
| } else { | |
| ce.logger.Debug().Msg("Skipping no-changes comment (show-no-changes-comment=false)") | |
| } | |
There was a problem hiding this comment.
@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)
There was a problem hiding this comment.
@Greyeye added your suggestion 🙏
normally when is the next release scheduled?
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
can you also add Caller() to the debug?
like
log.Debug().Caller()
…m/rvbsalgado/kubechecks into feat/466-hide-no-changes-comments Signed-off-by: rvbsalgado <rvbsalgado@users.noreply.github.com>
…m/rvbsalgado/kubechecks into feat/466-hide-no-changes-comments
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } else { | ||
| ce.logger.Debug().Msg("Skipping no-changes comment (show-no-changes-comment=false)") | ||
| } |
There was a problem hiding this comment.
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.
| } | |
| } | |
| // 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") | |
| } |
| 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") | ||
| } | ||
| } else { | ||
| ce.logger.Debug().Msg("Skipping no-changes comment (show-no-changes-comment=false)") | ||
| } |
There was a problem hiding this comment.
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).
Adds KUBECHECKS_SHOW_NO_CHANGES_COMMENT env var to control whether kubechecks posts a comment when no affected apps are found.
Closes issue #466