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

build: print out the number of warnings after completing a rule check #2647

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

daghack
Copy link
Contributor

@daghack daghack commented Aug 12, 2024

Atm when building with --check, when there are no warnings we generate the output
Check completed, no warnings found.

 => => transferring context: 88B                                                                                                                         0.0s
Check completed, no warnings found.

When there are warnings, we leap right into displaying them.

 => => transferring context: 88B 

WARNING: FromAsCasing - https://docs.docker.com/go/dockerfile/rule/from-as-casing/ ....

This updates the output in the later case to contain a similar "Check completed" message which lists the warning count.

 => => transferring context: 88B 
Check completed, 1 warning found!

WARNING: FromAsCasing - https://docs.docker.com/go/dockerfile/rule/from-as-casing/ ....

@daghack daghack self-assigned this Aug 12, 2024
@daghack daghack marked this pull request as draft August 12, 2024 05:24
commands/build.go Outdated Show resolved Hide resolved
@daghack daghack marked this pull request as ready for review August 12, 2024 13:46
Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

Can we have an integration test for both running check with and without warning to validate this?

commands/build.go Outdated Show resolved Hide resolved
@daghack
Copy link
Contributor Author

daghack commented Aug 14, 2024

Can we have an integration test for both running check with and without warning to validate this?

Added!

@daghack daghack requested a review from crazy-max August 15, 2024 13:22
@thompson-shaun thompson-shaun added this to the v0.17.0 milestone Aug 15, 2024
@daghack
Copy link
Contributor Author

daghack commented Aug 19, 2024

@crazy-max @tonistiigi ping

Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

@daghack Did you check the output of this for buildx bake as well?

@daghack
Copy link
Contributor Author

daghack commented Aug 19, 2024

@tonistiigi Good catch. Will update!

@crazy-max
Copy link
Member

@daghack Did you check the output of this for buildx bake as well?

Yes another integration for bake would be ideal to make sure

@daghack daghack force-pushed the print-warning-count branch 2 times, most recently from 2f99242 to eb4e3db Compare August 21, 2024 18:00
@daghack
Copy link
Contributor Author

daghack commented Aug 21, 2024

@crazy-max @tonistiigi Added matching integration checks for bake! :)

tests/bake.go Outdated Show resolved Hide resolved
tests/bake.go Outdated Show resolved Hide resolved
Signed-off-by: Talon Bowler <talon.bowler@docker.com>
@tonistiigi tonistiigi merged commit b7486e5 into docker:master Sep 3, 2024
106 checks passed
@daghack daghack deleted the print-warning-count branch September 4, 2024 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants