Print lint warnings #4761
Merged
Print lint warnings #4761
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
daghack
force-pushed
the
print-lint-warnings
branch
from
3d98105 to
0ed1a23
Compare
Closed
daghack
force-pushed
the
print-lint-warnings
branch
from
c56ebda to
51b246b
Compare
Signed-off-by: Talon Bowler <[email protected]>
daghack
force-pushed
the
print-lint-warnings
branch
from
51b246b to
72d7aa7
Compare
tonistiigi
reviewed
Apr 9, 2024
| return res, nil | ||
| } | ||
|
|
||
| func PrintLintViolations(dt []byte, w io.Writer) error { |
There was a problem hiding this comment.
@colinhemmings Any comments about the output here?
daghack
force-pushed
the
print-lint-warnings
branch
from
f5fa4fd to
aec4bda
Compare
Signed-off-by: Talon Bowler <[email protected]>
daghack
force-pushed
the
print-lint-warnings
branch
from
aec4bda to
81fc6a3
Compare
tonistiigi
reviewed
Apr 10, 2024
daghack
force-pushed
the
print-lint-warnings
branch
from
ced11ee to
543112e
Compare
|
@tonistiigi Ready for a second review, at your leisure! 😃 |
daghack
force-pushed
the
print-lint-warnings
branch
from
543112e to
c772ed1
Compare
tonistiigi
reviewed
Apr 11, 2024
| fmt.Fprintf(tw, "\t%s:%d\n", source.Filename, firstRange.Start.Line) | ||
| } | ||
| fmt.Fprintf(tw, "\t%s\n", warning.Detail) | ||
| for _, r := range warning.Location.Ranges { |
There was a problem hiding this comment.
Follow-up:
The source stacktrace doesn't seem to match the style in the progressbar:
Lint Rule FromAsCasing
Dockerfile:1
Lint Rule 'FromAsCasing': 'as' and 'FROM' keywords' casing do not match (line 1)
1 | FROM alpine as Base
- Lint Rule 'FromAsCasing': 'as' and 'FROM' keywords' casing do not match (line 1)
The 'as' keyword should match the case of the 'from' keyword
Dockerfile:1
--------------------
1 | >>> FROM alpine as Base
2 | RUN apk add git
3 | workdir /
--------------------
There was a problem hiding this comment.
Also, note that this description part is missing in print output. Is this intentional?
There was a problem hiding this comment.
It ended up being a lot of redundant information displayed and looked pretty cluttered. That being said, getting them to match the much superior progressbar style is an EXCELLENT idea. 😂
Signed-off-by: Talon Bowler <[email protected]>
daghack
force-pushed
the
print-lint-warnings
branch
from
c772ed1 to
e81c6c6
Compare
tonistiigi
approved these changes
Apr 11, 2024
This was referenced Apr 11, 2024
This was referenced Apr 15, 2024
minamir85
approved these changes
Jun 18, 2024
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.
Following on the footsteps of #4759, this adds support for an additional subrequest frontends can choose to implement for exposing linting errors and implements this subrequest for the
dockerfilefrontend.