Skip to content

Add disable-command-output-logging hook property - #761

Open
arjunxplorer wants to merge 1 commit into
adnanh:masterfrom
arjunxplorer:disable-command-output-logging
Open

Add disable-command-output-logging hook property#761
arjunxplorer wants to merge 1 commit into
adnanh:masterfrom
arjunxplorer:disable-command-output-logging

Conversation

@arjunxplorer

Copy link
Copy Markdown

Summary

  • Adds per-hook disable-command-output-logging boolean property
  • When true, suppresses command output: log line on successful executions
  • Errors are still logged (backward compatible default: false)

Fixes #680

Test plan

  • go test -run TestDisableCommandOutputLogging -v
  • go test ./...

Fixes adnanh#680. Adds a per-hook boolean to skip logging command output on
successful executions while still logging on errors. Defaults to false
for backward compatibility.
@arjunxplorer

Copy link
Copy Markdown
Author

Hi @adnanh,
The test failure appears to be a macOS runner environment issue (dyld linking errors), not related to the code changes in this PR.

Could you please try to retry the workflow to see if it passes on the next attempt?

Thank you!

@adnanh adnanh left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The implementation looks correct and preserves existing behavior: false remains the default, so existing configurations continue logging command output.

One requested change: please document disable-command-output-logging in docs/Hook-Definition.md, including that it defaults to false, suppresses output only for successful commands, and still logs output when execution fails.

Non-blocking: consider testing this through actual JSON and YAML hook configurations. The current tests set the Go field directly, so they do not verify that the public configuration key is parsed correctly.

Also, consider restoring the previous log.Writer() in the test rather than assuming os.Stderr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add hook property to control command output logging

2 participants