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

Warn on empty continuation lines only, not on comment-only lines #35004

Merged
merged 1 commit into from Sep 28, 2017

Conversation

Projects
None yet
6 participants
@thaJeztah
Member

thaJeztah commented Sep 27, 2017

Alternative approach to #34333

Commit 8d1ae76 added deprecation warnings for empty continuation lines, but also treated comment-only lines as empty.

This patch distinguishes empty continuation lines from comment-only lines, and only outputs warnings for the former.

ping @dnephin @tianon

Warn on empty continuation lines only, not for comments
Commit 8d1ae76 added
deprecation warnings for empty continuation lines,
but also treated comment-only lines as empty.

This patch distinguishes empty continuation lines
from comment-only lines, and only outputs warnings
for the former.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Sep 27, 2017

Member

@tianon done 👍

Member

thaJeztah commented Sep 27, 2017

@tianon done 👍

@tianon

tianon approved these changes Sep 27, 2017

😍

@coredumperror

This comment has been minimized.

Show comment
Hide comment
@coredumperror

coredumperror Sep 27, 2017

Looks like a much better solution than mine. It's much clearer what the patch is changing, and why.

coredumperror commented Sep 27, 2017

Looks like a much better solution than mine. It's much clearer what the patch is changing, and why.

@dnephin

LGTM

@tonistiigi tonistiigi merged commit 94b9870 into moby:master Sep 28, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 37135 has succeeded
Details
janky Jenkins build Docker-PRs 45811 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 6192 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 17374 has succeeded
Details
z Jenkins build Docker-PRs-s390x 5984 has succeeded
Details

@thaJeztah thaJeztah deleted the thaJeztah:dont-warn-for-comment-only-lines branch Sep 28, 2017

dgroup pushed a commit to dgroup/threecopies that referenced this pull request Sep 29, 2017

dubinka
All `RUN` commands were joined in order to minimize the layers size.
During docker image building procedure you may observe warning `[WARNING]: Empty continuation line found`.
This is docker bug, see more details here:
 - moby/moby#35004
 - appropriate/docker-jetty#79
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 3, 2017

Member

For future reference; this patch was included in docker 17.10 docker/docker-ce@f6d296e

Member

thaJeztah commented Nov 3, 2017

For future reference; this patch was included in docker 17.10 docker/docker-ce@f6d296e

@tianon

This comment has been minimized.

Show comment
Hide comment
@tianon

tianon Nov 3, 2017

Member

Since 17.09 is a "stable" release, would it make sense to add this to the pile of potential-backport-candidates for a future 17.09.1? 😇

Member

tianon commented Nov 3, 2017

Since 17.09 is a "stable" release, would it make sense to add this to the pile of potential-backport-candidates for a future 17.09.1? 😇

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 3, 2017

Member

@tianon I'll add it to the pile for consideration, but it may be considered "non-critical" as the issue doesn't break the build (but, yes, it's confusing/annoying)

Member

thaJeztah commented Nov 3, 2017

@tianon I'll add it to the pile for consideration, but it may be considered "non-critical" as the issue doesn't break the build (but, yes, it's confusing/annoying)

@tianon

This comment has been minimized.

Show comment
Hide comment
@tianon

tianon Nov 3, 2017

Member

Thanks! Definitely agree with that assessment -- I just want to make sure it's in the pile of things being considered if something critical enough to warrant a 17.09.1 does come up. 👍

Member

tianon commented Nov 3, 2017

Thanks! Definitely agree with that assessment -- I just want to make sure it's in the pile of things being considered if something critical enough to warrant a 17.09.1 does come up. 👍

@coredumperror

This comment has been minimized.

Show comment
Hide comment
@coredumperror

coredumperror Nov 3, 2017

Hurray! I just updated to the latest docker-for-mac edge build, and can confirm that I no longer get giant sees of red text in my docker build output!

coredumperror commented Nov 3, 2017

Hurray! I just updated to the latest docker-for-mac edge build, and can confirm that I no longer get giant sees of red text in my docker build output!

AlexGoico added a commit to AlexGoico/docker-pygit2 that referenced this pull request Nov 15, 2017

Removed whitespace that would emit warnings
There are warnings that would be emitted because of empty line continuation characters. This commit should eliminate the warnings in the next release of docker. See: moby/moby#35004

mrled added a commit to mrled/psyops that referenced this pull request Jan 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment