Skip to content
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.

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

Fix error when hook cannot be run due to missing interpreter #2948

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

mcncl
Copy link
Contributor

@mcncl mcncl commented Aug 26, 2024

Description

There is a missing value in the formatted string for the error, it is passing the interpreter as the string value for the hook name, and is then printing an empty %q for the interpreter value.

Changes

There is an addition of the hookName passed as a string to the logMissingHookInfo function. This value is passed from the runWrappedShellScriptHook function on 534.

Testing

  • Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
  • Code is formatted (with go fmt ./...)

passing the interpreter as the string value for the hook name, and is
then printing an empty `%q` for the interpreter value.

There is an addition of the `hookName` passed as a string to the
`logMissingHookInfo` function. This value is passed from the
`runWrappedShellScriptHook` function on [534](https://github.com/buildkite/agent/blob/65ed6aaf3dd3e50f88dee3fbd27714120e558965/internal/job/executor.go#L534).
@mcncl mcncl changed the title There is a missing value in the formatted string for the error, it is Fix error when hook cannot be run Aug 26, 2024
@mcncl mcncl changed the title Fix error when hook cannot be run Fix error when hook cannot be run due to missing interpreter Aug 26, 2024
Copy link
Contributor

@DrJosh9000 DrJosh9000 left a comment

Choose a reason for hiding this comment

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

Thanks! Good catch!

@mcncl mcncl enabled auto-merge August 26, 2024 04:38
@mcncl mcncl merged commit 7173de0 into buildkite:main Aug 26, 2024
1 check passed
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.

2 participants