-
Notifications
You must be signed in to change notification settings - Fork 21
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
github: annotate build warnings #365
Conversation
44e2a6e
to
aa26ffd
Compare
This comment was marked as outdated.
This comment was marked as outdated.
aa26ffd
to
0ffd26f
Compare
6d9b5c0
to
653541f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
f94f0e7
to
f66a1cc
Compare
c311ee7
to
dc41dc9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
@@ -15,7 +15,7 @@ on: | |||
|
|||
env: | |||
NODE_VERSION: "20" | |||
BUILDX_VERSION: "v0.15.1" | |||
BUILDX_VERSION: "https://github.com/docker/buildx.git#d8c9ebde1fdcf659f1fa3efa6ccc27a28b0f1564" # https://github.com/docker/buildx/pull/2551 |
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.
needs follow-up to bump to v0.16.0 when released
dc41dc9
to
ab12e51
Compare
src/github.ts
Outdated
let startLine: number | undefined, endLine: number | undefined; | ||
for (const range of warning.range ?? []) { | ||
if (range.start.line && (!startLine || range.start.line < startLine)) { | ||
startLine = range.start.line; | ||
} | ||
if (range.end.line && (!endLine || range.end.line > endLine)) { | ||
endLine = range.end.line; | ||
} | ||
} | ||
core.info(`${message} --- startLine: ${startLine}, endLine: ${endLine}`); | ||
core.warning(message, { | ||
title: title, | ||
file: source, | ||
startLine: startLine, | ||
endLine: endLine | ||
}); |
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.
Was looking at showing first and last lines matching warning ranges but GitHub annotations does not seem to display that properly:
core.info
returns the right startLine
and endLine
though:
ConsistentInstructionCasing: Command 'COPy' should match the case of the command majority (lowercase)
More info: https://docs.docker.com/go/dockerfile/rule/consistent-instruction-casing/ --- startLine: 21, endLine: 23
I will just set startLine
for now and open a thread on GitHub community.
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.
42c7667
to
1cee406
Compare
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
1cee406
to
644587f
Compare
let message = atob(warning.short).replace(/\s\(line \d+\)$/, ''); | ||
if (warning.url) { | ||
// https://github.com/docker/buildx/blob/d8c9ebde1fdcf659f1fa3efa6ccc27a28b0f1564/commands/build.go#L854 | ||
message += `\nMore info: ${warning.url}`; |
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.
Wonder if this link can be made clickable by adding some markdown around it.
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.
Opened https://github.com/orgs/community/discussions/129897
And does not work with markdown 😞 https://github.com/orgs/community/discussions/129897#discussioncomment-9886696
1fc5c48
to
644587f
Compare
needs buildx(build): resolveWarnings from metadata #362example in this PR through integration tests: https://github.com/docker/actions-toolkit/pull/365/files#diff-cc857d138abc3f09c4d212a933d59584a266fe71521fd2cd87327dc037558a43R17