-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Updates lint output to print detail instead of description #5045
Conversation
1de23db
to
71854d5
Compare
We probably need to go over these more in some follow-ups. Looking at the example I would say in that case both messages are useful. One is pointing out a mistake and suggesting a fix and the other is describing what the general rule for the validation is. I guess for some of the rules the separation isn't quite as obvious and we don't show both to avoid duplication. In that case I think we can define what rules benefit from multiple fields and keep it hidden/empty if extra field does not add additional context. We can also define more clearly how the verbosity changes with |
This removes the line position from the default output for build warnings (without --debug). Before:
Now:
|
71854d5
to
fd36d4a
Compare
Signed-off-by: Talon Bowler <[email protected]>
fd36d4a
to
9305c60
Compare
Updated to return the line number to the |
func LintFormatShort(rulename, msg string, line int) string { | ||
msg = fmt.Sprintf("%s: %s", rulename, msg) | ||
if line > 0 { | ||
msg = fmt.Sprintf("%s (line %d)", msg, line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tonistiigi Is it needed to have the line number in progress as we already have this information in?:
Usage of undefined variable '$PAHT' (did you mean $PATH?)
Dockerfile:3
--------------------
1 | FROM alpine
2 | >>> ENV PATH=$PAHT:/foo
3 |
--------------------
Mostly looking at docker/actions-toolkit#365 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is only with --debug
. By default there is no source code view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this is not needed for example in your github actions excerpt. It should probably be added in buildx side for views that need it, not inside the string here.
Currently, the
PrintLintViolations
output warning text includes the static description of the lint rule, rather than the formatted text message that often includes context specific to a given instance of rule violation.This updates
PrintLintViolations
to print out the formatted lint message instead. Below is an instance where you can see the difference in the types of output.Current:
New: