Skip to content

Conversation

bthomee
Copy link
Collaborator

@bthomee bthomee commented Aug 18, 2025

High Level Overview of Change

This change replaces the use of the tee command to >> instead.

Context of Change

Some of the workflows write more than one output to ${GITHUB_OUTPUT}, whereby currently the last output overwrites all previous outputs. Using >> is less error-prone than tee in that situation.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

Copy link
Contributor

@mathbunnyru mathbunnyru left a comment

Choose a reason for hiding this comment

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

@bthomee
Copy link
Collaborator Author

bthomee commented Aug 18, 2025

Let's use >> "$GITHUB_OUTPUT"

That's what Github docs do too: https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#example-masking-a-generated-output-within-a-single-job

That's what I used to have in my refactor CI PR, until I saw in our previous CI jobs that tee was used a lot. After doing some debugging during development, I've come to appreciate the use of tee since it also prints the actual value that's being set as output to stdout. As such, I find it actually valuable.

I don't mind switching to >> but it would have to be accompanied by an echo statement that also prints out the value to stdout, so it's available in the logs.

@mathbunnyru
Copy link
Contributor

Let's use >> "$GITHUB_OUTPUT"
That's what Github docs do too: docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#example-masking-a-generated-output-within-a-single-job

That's what I used to have in my refactor CI PR, until I saw in our previous CI jobs that tee was used a lot. After doing some debugging during development, I've come to appreciate the use of tee since it also prints the actual value that's being set as output to stdout. As such, I find it actually valuable.

I don't mind switching to >> but it would have to be accompanied by an echo statement that also prints out the value to stdout, so it's available in the logs.

If you need some values for debug purposes, it's easy enough to enable the output by setting set -x temporarily, so I think the best way is to use >> and then if needed you'll be able to debug by adding a single line of code

@bthomee bthomee changed the title fix: Modify jobs to append to GITHUB_OUTPUT fix: Modify jobs to use '>>' instead of 'tee' for GITHUB_OUTPUT Aug 18, 2025
@bthomee
Copy link
Collaborator Author

bthomee commented Aug 18, 2025

If you need some values for debug purposes, it's easy enough to enable the output by setting set -x temporarily, so I think the best way is to use >> and then if needed you'll be able to debug by adding a single line of code

Done.

@bthomee bthomee added the Trivial Simple change with minimal effect, or already tested. Only needs one approval. label Aug 18, 2025
@bthomee bthomee merged commit b04d239 into develop Aug 18, 2025
18 of 31 checks passed
@bthomee bthomee deleted the bthomee/append branch August 18, 2025 14:49
ximinez added a commit that referenced this pull request Aug 18, 2025
…actoring-1

* XRPLF/develop:
  fix: Modify jobs to use '>>' instead of 'tee' for GITHUB_OUTPUT (#5699)
  refactor: Revamp CI workflows (#5661)
  refactor: Decouple net from xrpld and move rpc-related classes to the rpc folder (#5477)
  Set version to 2.6.0-rc2
  docs: Updates list of maintainers and reviewers (#5687)
  fix: Change log to debug level for AMM offer retrieval and IOU payment check (#5686)
  fix: Add -Wno-deprecated-declarations for Clang only (#5680)
  Update .git-blame-ignore-revs for #5657 (#5675)
  Fix BUILD.md instruction (#5676)
  Set version to 2.6.0-rc1
  fix: Improve logging of the reason to refuse a peer connection (#5664)
  fix: Make test suite names match the directory name (#5597)
  chore: Run prettier on all files (#5657)
  chore: Set CONAN_REMOTE_URL also for forks (#5662)
  chore: Cleanup bin/ directory (#5660)
  perf: Optimize hash performance by avoiding allocating hash state object (#5469)
ximinez added a commit that referenced this pull request Aug 18, 2025
…to ximinez/lending-XLS-66

* XRPLF/ximinez/lending-refactoring-4:
  fix: Modify jobs to use '>>' instead of 'tee' for GITHUB_OUTPUT (#5699)
  refactor: Revamp CI workflows (#5661)
  refactor: Decouple net from xrpld and move rpc-related classes to the rpc folder (#5477)
  Set version to 2.6.0-rc2
  docs: Updates list of maintainers and reviewers (#5687)
  fix: Change log to debug level for AMM offer retrieval and IOU payment check (#5686)
  fix: Add -Wno-deprecated-declarations for Clang only (#5680)
  Update .git-blame-ignore-revs for #5657 (#5675)
  Fix BUILD.md instruction (#5676)
  Set version to 2.6.0-rc1
  fix: Improve logging of the reason to refuse a peer connection (#5664)
  fix: Make test suite names match the directory name (#5597)
  chore: Run prettier on all files (#5657)
  chore: Set CONAN_REMOTE_URL also for forks (#5662)
  chore: Cleanup bin/ directory (#5660)
  perf: Optimize hash performance by avoiding allocating hash state object (#5469)
yinyiqian1 pushed a commit to yinyiqian1/rippled that referenced this pull request Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Trivial Simple change with minimal effect, or already tested. Only needs one approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants