Skip to content
This repository has been archived by the owner on Oct 2, 2024. It is now read-only.

reprint warnings before exit (ch-run, ch-image) #1677

Merged
merged 26 commits into from
Sep 6, 2023
Merged

Conversation

lucaudill
Copy link
Collaborator

closes #1674

@lucaudill lucaudill marked this pull request as ready for review July 24, 2023 19:57
@lucaudill lucaudill requested a review from reidpr July 24, 2023 19:57
Copy link
Collaborator

@reidpr reidpr left a comment

Choose a reason for hiding this comment

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

Good stuff! Suggestions in-line.

bin/ch-run.c Show resolved Hide resolved
bin/ch-run.c Outdated Show resolved Hide resolved
bin/ch_misc.c Outdated Show resolved Hide resolved
bin/ch_misc.c Outdated Show resolved Hide resolved
bin/ch_misc.c Outdated Show resolved Hide resolved
bin/ch_misc.c Outdated Show resolved Hide resolved
bin/ch_misc.c Outdated Show resolved Hide resolved
bin/ch_misc.c Show resolved Hide resolved
lib/charliecloud.py Outdated Show resolved Hide resolved
test/build/50_dockerfile.bats Show resolved Hide resolved
@lucaudill lucaudill requested a review from reidpr August 29, 2023 23:07
Copy link
Collaborator

@reidpr reidpr left a comment

Choose a reason for hiding this comment

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

Great work! See tidy commit, which builds but is mostly untested and will surely fail CI, so fixing that remains.

Comment on lines 1067 to 1069
# Warnings list is a statically sized memory buffer. Ensure it works as
# intended by printing more warnings than can be saved to this buffer and
# checking that the program doesn’t crash.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also validate the number of warnings re-printed.

Comment on lines +1077 to +1079
[[ $output == *'ch-run['*']: warning: reprinting first '*' warning(s)'* ]]
[[ $(echo "$output" | grep -Fc 'this is warning 1!') -eq 2 ]]
[[ $(echo "$output" | grep -Fc 'this is warning 100!') -eq 1 ]]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Clever!

@reidpr reidpr merged commit d2dbb00 into master Sep 6, 2023
6 checks passed
@reidpr reidpr deleted the reprint-warnings_1674 branch September 6, 2023 21:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

re-print warnings before exit
2 participants