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

Update printing style #252

Merged
merged 10 commits into from
Dec 19, 2024
Merged

Update printing style #252

merged 10 commits into from
Dec 19, 2024

Conversation

schneems
Copy link
Contributor

This updates the Procfile to the bullet_stream style of bulletpoint text. The Output has changed to include the command being pulled from the Procfile

@schneems schneems marked this pull request as ready for review December 17, 2024 04:21
@schneems schneems requested a review from a team as a code owner December 17, 2024 04:21
@schneems schneems enabled auto-merge (squash) December 17, 2024 19:33
CHANGELOG.md Outdated Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
@runesoerensen
Copy link
Contributor

runesoerensen commented Dec 19, 2024

I like that this PR adds the Procfile commands to the output, and matches the style of other buildpacks better. The new output drops some of the output that describes what's happening though (and the empty Procfile output seems a bit more awkward).

Current output

// Empty Procfile
[Discovering process types]
Procfile declares types -> (none)

// Procfile defining worker and console
[Discovering process types]
Procfile declares types -> worker, console

New output

// Empty Procfile
## Procfile Buildpack
- Processes
    - (none)
- Done (finished in < 0.1s)

// Procfile defining worker and console
## Procfile Buildpack
- Processes
    - worker: `echo 'this is the worker process!'`
    - console: `echo 'this is the console process!'`
- Done (finished in < 0.1s)

Not a blocker, but would it perhaps make sense to consider ways the new output can retain some of the expressiveness of the current output, and still fit the new style? Maybe something like:

// Empty Procfile
## Procfile Buildpack
- Discovering process types
- Procfile is empty
- Done (finished in < 0.1s)

// Procfile defining worker and console
## Procfile Buildpack
- Discovering process types
- Detected processes
    - worker: `echo 'this is the worker process!'`
    - console: `echo 'this is the console process!'`
- Done (finished in < 0.1s)

or

// Empty Procfile
## Procfile Buildpack
- Discovering process types
    - Procfile is empty
- Done (finished in < 0.1s)

// Procfile defining worker and console
## Procfile Buildpack
- Discovering process types
    - worker: `echo 'this is the worker process!'`
    - console: `echo 'this is the console process!'`
- Done (finished in < 0.1s)

By default Rust's `fs` module does not include the filename when an error is returned. The `fs_err` crate adds filenames back in.
While the name of the buildpack "Procfile Buildpack" hints that it's looking at a Procfile, be specific with the value.
The output `(none)` is not a full sentence. This explains that the buildpack saw an empty file (not entirely true, it could be an error or mis-parse, but there's an open issue for that) and as a result defined no processes.
@schneems schneems disabled auto-merge December 19, 2024 15:17
@schneems schneems merged commit 9a42bcf into main Dec 19, 2024
4 checks passed
@schneems schneems deleted the schneems/bullet-print branch December 19, 2024 23:32
heroku-linguist bot added a commit that referenced this pull request Dec 20, 2024
## heroku/procfile

### Changed

- Build output style updated to `bullet_stream`. Output now also includes the commands that were pulled from the `Procfile` in the output. ([#252](#252))
@heroku-linguist heroku-linguist bot mentioned this pull request Dec 20, 2024
heroku-linguist bot added a commit to heroku/cnb-builder-images that referenced this pull request Dec 20, 2024
## heroku/procfile

### Changed

- Build output style updated to `bullet_stream`. Output now also includes the commands that were pulled from the `Procfile` in the output. ([#252](heroku/buildpacks-procfile#252))
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.

3 participants