Get rid of fluttered build job output and color errors #374
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ammernico
force-pushed
the
exit-status-colors
branch
2 times, most recently
from
063866d
to
0161414
Compare
ammernico
force-pushed
the
exit-status-colors
branch
3 times, most recently
from
2339aef
to
a337e8a
Compare
ammernico
changed the title
ammernico
force-pushed
the
exit-status-colors
branch
from
a337e8a
to
8ef121d
Compare
|
tested updated version, looks much better now - thanks! |
ammernico
force-pushed
the
exit-status-colors
branch
3 times, most recently
from
65bb663
to
bdb7f3e
Compare
primeos-work
requested changes
May 23, 2024
|
@christophprokop can you perhaps also provide motivation/background for or rather reasoning behind the output format changes? The things that seem obvious:
What I'm not sure about:
|
ammernico
force-pushed
the
exit-status-colors
branch
4 times, most recently
from
63d5797
to
f6d11a5
Compare
ammernico
force-pushed
the
exit-status-colors
branch
from
f6d11a5
to
770f570
Compare
primeos-work
requested changes
Jun 5, 2024
Comment on lines
+596
to
+597
| "", | ||
| "", |
There was a problem hiding this comment.
Is that information (endpoint name + container ID) not available here? In that case we should perhaps use a placeholder like dashes (that also helps with keeping the output structured/parseable).
There was a problem hiding this comment.
This is the placeholder with dashes. The given string here "" gets filled up with dashes.
Looks a bit odd, but works
ammernico
force-pushed
the
exit-status-colors
branch
from
770f570
to
2a95ed9
Compare
This commit enhances the finish message formatting by incorporating color-coded status messages for improved readability and clarity. Changes include: - Replaced redundant format strings with simple colorized messages. - Consolidated the final message formatting to a single step with colored status indicators. Key modifications: - Used `colored::Colorize` for colorizing the success and error messages. - Simplified message construction by combining dynamic and static parts after determining the status. Signed-off-by: Nico Steinle <nico.steinle@eviden.com>
This update enhances the readability of the build output by
standardizing the format and reducing text flutter when building
multiple packages. Key changes include:
- Implemented dynamic length calculation for endpoint names to ensure
proper alignment and prevent text cutoff.
- Introduced color-coded build status blocks for better visual
distinction.
- Updated progress bar messages to use a consistent format,
accommodating longer endpoint names and maintaining a clean display.
Example output before and after the changes:
```
- before:
[00:00:06] (100%): █████ | [endpoint-name/XXXXXXX XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX package-name package_version]: msg
- after:
[00:00:06] (100%): █████ | endpoint-name XXXXXXX XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX ██ package_name package_version msg
^^--- colored build status block
```
Signed-off-by: Nico Steinle <nico.steinle@eviden.com>
This commit refines the progress bar format in both the `config.toml` file and the `default_progress_format` function to improve readability and adapt to narrower terminal widths. The changes include: - Simplified the progress format by removing unnecessary brackets and adjusting the bar length. - Reduced the progress bar length from 40 characters to 5 characters to fit within more compact terminal displays. Signed-off-by: Nico Steinle <nico.steinle@eviden.com>
ammernico
force-pushed
the
exit-status-colors
branch
from
2a95ed9
to
8369521
Compare
primeos-work
approved these changes
Jun 6, 2024
There was a problem hiding this comment.
LGTM now, thanks for writing proper commit messages :)
Having to pass through max_endpoint_name_length isn't pretty but I guess it's the best solution/hack for the meantime.
I tested it and the output seems fine. (I would've preferred to keep some additional information in some places (like Phases:) but that's down to personal preference, I guess.)
Merged
via the queue into
science-computing:master
with commit Jun 6, 2024
2f085fb
12 of 13 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.