Skip to content

fix: print output from post render hook in console when withChange: false#9258

Merged
renzodavid9 merged 2 commits into
GoogleContainerTools:mainfrom
renzodavid9:render-hooks-stdout
Jan 18, 2024
Merged

fix: print output from post render hook in console when withChange: false#9258
renzodavid9 merged 2 commits into
GoogleContainerTools:mainfrom
renzodavid9:render-hooks-stdout

Conversation

@renzodavid9
Copy link
Copy Markdown
Contributor

@renzodavid9 renzodavid9 commented Jan 17, 2024

Fixes: #9262

Description

  • This PR sends stdout to the post-render hooks, when withChange: false, so user can print content from their associated script and see it in console

  • From

    if err := setUpLogs(errOut, v, timestamps); err != nil {
    the logger is configured to use stderr, so we can get the writer from it to print the content

  • Decided to get the writer from the logger instead of calling os.Stderr directly so we share the same behaviour in case the logger changes, e.g., printing to a document (that's something that can not do today)

  • Changed back the Starting post-render hooks..., etc, messages to use output.Default.Fprintln so we print the ERRO label

Follow-up Work

  • A good idea might be to send both, stdout and stderr to the post-render hooks when withChange: true so users can print info content + the rendered manifests

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 17, 2024

Codecov Report

Attention: 295 lines in your changes are missing coverage. Please review.

Comparison is base (290280e) 70.48% compared to head (5131626) 63.64%.
Report is 1094 commits behind head on main.

Files Patch % Lines
cmd/skaffold/app/cmd/exec.go 16.32% 40 Missing and 1 partial ⚠️
cmd/skaffold/app/cmd/filter.go 47.27% 22 Missing and 7 partials ⚠️
cmd/skaffold/app/cmd/lsp.go 28.12% 23 Missing ⚠️
cmd/skaffold/app/cmd/verify.go 23.33% 23 Missing ⚠️
cmd/skaffold/app/cmd/fix.go 51.16% 17 Missing and 4 partials ⚠️
cmd/skaffold/app/cmd/inspect_job_manifest_paths.go 60.00% 15 Missing and 1 partial ⚠️
cmd/skaffold/app/cmd/inspect_namespaces.go 50.00% 13 Missing and 1 partial ⚠️
...md/skaffold/app/cmd/inspect_config_dependencies.go 45.83% 12 Missing and 1 partial ⚠️
cmd/skaffold/app/cmd/lint.go 42.85% 12 Missing ⚠️
cmd/skaffold/app/cmd/inspect_build_env.go 60.71% 11 Missing ⚠️
... and 21 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9258      +/-   ##
==========================================
- Coverage   70.48%   63.64%   -6.85%     
==========================================
  Files         515      632     +117     
  Lines       23150    32607    +9457     
==========================================
+ Hits        16317    20752    +4435     
- Misses       5776    10258    +4482     
- Partials     1057     1597     +540     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pull-request-size pull-request-size Bot added size/M and removed size/S labels Jan 17, 2024
@renzodavid9 renzodavid9 marked this pull request as ready for review January 18, 2024 18:45
@renzodavid9 renzodavid9 merged commit ec32c30 into GoogleContainerTools:main Jan 18, 2024
@renzodavid9 renzodavid9 deleted the render-hooks-stdout branch January 18, 2024 19:29
@menahyouyeah menahyouyeah mentioned this pull request Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Output from post render hook not printed in console

2 participants