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

[stacked] Finalize bullet_stream output update #769

Open
wants to merge 25 commits into
base: malax/output
Choose a base branch
from

Conversation

schneems
Copy link
Contributor

@schneems schneems commented Jan 31, 2025

Carries #745 over the line so the new output format can be merged. As opposed to #767 this is intended to be a minimal change. It does not resolve #762. It does not update the jvm-function-invoker buildpack's output.

Commits are targeted and intended to be reviewed individually, in order.

Right now the task name is so long you can't see which guide is being executed without clicking through. This PR refactors the task so that the name is shorter (by making the task name shorter and removing arch variables from the matrix.
Uses the newly defined `output` interface that Manuel introduced for the JVM buildpack in #745 instead of the legacy libherokubuildpack::log::log_error. For all buildpacks except for jvm-function-invoker.
The trailing whitespace after the `! ` shouldn't be significant.
@schneems schneems changed the title Finalize bullet_stream output update [stacked] Finalize bullet_stream output update Jan 31, 2025
The `track_timing` interface (from the name alone) isn't clear that it produces a sub-bullet output (it wasn't actually clear that it didn't produce dots `.....`). There are places where it's used incorrectly:

```
- Running `./mvnw dependency:list` quietly
  - Done (7.6s)
```

This change makes it harder to use wrong. Every streamed command has timing information emitted, guaranteed, so there's never a mistake of output from one command bleeding into another by accident.
This doesn't use the bullet_stream "dots" feature (which I don't think is implemented here) but it does emit the timing information on the next line to indicate that it finished.
```
  - Done (4.4s)
Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF-8
- Done (finished in 34.0s)
```
@schneems schneems added the skip changelog Pull requests that do not require changes to the CHANGELOG file label Jan 31, 2025
@schneems schneems force-pushed the schneems/bs_output_ready_for_review branch from d34f43c to f02864f Compare January 31, 2025 23:13
@schneems schneems marked this pull request as ready for review February 1, 2025 18:50
@schneems schneems requested a review from Malax as a code owner February 1, 2025 18:50
@schneems schneems requested a review from a team as a code owner February 1, 2025 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip changelog Pull requests that do not require changes to the CHANGELOG file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant