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.
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: display Dockerfile path on check warnings #2672
build: display Dockerfile path on check warnings #2672
Changes from all commits
f1b92e9
671bd1b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
What will be the output for
inline
andstdin
case? I guess it should be omitted like:Something like this would be confusing:
If someone has a Dockerfile named
inline
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.
Removing the line entirely requires either a
buildkit
PR or refactoring howbuildx
is printing warnings to effectively duplicate that functionality. I'm not opposed, but if this is a strong ask then it might be best to merge this and then make a corresponding issue inbuildkit
for the follow-up work.I was trying to model what @tonistiigi had suggested, here. For MY part, I think that ambiguity around two very specifically named Dockerfiles (in this case,
stdin
andinline
) is less confusing than simply not showing anything in these cases. That being said, it could be updated to something a little more clear and even less likely to be a filename?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.
Yes that sounds good, let's check that for a follow-up then