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

Separate helm notes #92

Open
wants to merge 3 commits into
base: 3.x
Choose a base branch
from

Conversation

theturtle32
Copy link
Contributor

@theturtle32 theturtle32 commented Nov 19, 2024

Building on the Multiline Output fix in #85, this PR splits helm's NOTES.txt output from the rest of the output, and populates separate Github Actions outputs for each. This makes it easier to render the NOTES.txt into the Markdown-formatted workflow summary.

PR #85 should be merged first, as this PR is dependent upon it and includes its changes as well.

The two outputs are:

  • helm_output – Everything that was output by the helm command before the line NOTES:
  • helm_notes – Everything that was output by the helm command after the line NOTES:

I also updated the README to document the new outputs.

@WyriHaximus WyriHaximus added this to the v5.0.0 milestone Nov 19, 2024
@WyriHaximus
Copy link
Owner

@theturtle32 Merged your other PR's and released them in v4.0.2 an hour ago. Do you want to do anything to this PR before I review it? Put this up for v5 as it's a breaking change.

@theturtle32
Copy link
Contributor Author

@theturtle32 Merged your other PR's and released them in v4.0.2 an hour ago. Do you want to do anything to this PR before I review it? Put this up for v5 as it's a breaking change.

Is it really any more of a breaking change than the other PR that removed the escaping from the undocumented helm_output output?

If it would help, we could leave that one alone without modification, and just add two more, so you would have three outputs:

  • helm_output - the same as it is in the v4 release series: all the output from the helm command, including notes.
  • helm_notes - new: just the part after NOTES: in the helm output.
  • helm_debug - new: just the part before NOTES: in the helm output.

This would make it not a "breaking" change and allow for it to be released in the 4.x series.

Thoughts? Opinions on naming?

@WyriHaximus
Copy link
Owner

@theturtle32 Merged your other PR's and released them in v4.0.2 an hour ago. Do you want to do anything to this PR before I review it? Put this up for v5 as it's a breaking change.

Is it really any more of a breaking change than the other PR that removed the escaping from the undocumented helm_output output?

Honestly, I considered that a bug fix to a feature. Strictly speaking, it was undocumented and making any changes to it isn't a BC break.

If it would help, we could leave that one alone without modification, and just add two more, so you would have three outputs:

  • helm_output - the same as it is in the v4 release series: all the output from the helm command, including notes.
  • helm_notes - new: just the part after NOTES: in the helm output.
  • helm_debug - new: just the part before NOTES: in the helm output.

This would make it not a "breaking" change and allow for it to be released in the 4.x series.

Works for me 👍

Thoughts? Opinions on naming?

Well debug seems off to what is does, what about result?

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

Successfully merging this pull request may close these issues.

2 participants