Update "auto-generate" comments to improve detection by linters by thaJeztah · Pull Request #40077 · moby/moby

thaJeztah · 2019-10-12T20:01:16Z

As described in https://golang.org/s/generatedcode, Go has a formalized format that should be used to indicate that a file is generated.

Matching that format helps linters to skip generated files;

From https://golang.org/s/generatedcode (golang/go#13560 (comment));

Generated files are marked by a line of text that matches the regular expression, in Go syntax:

^// Code generated .* DO NOT EDIT.$

The .* means the tool can put whatever folderol it wants in there, but the comment
must be a single line and must start with Code generated and end with DO NOT EDIT.,
with a period.

The text may appear anywhere in the file.

This patch updates the autogenerated code to match that format.

thaJeztah · 2019-10-12T20:04:28Z

hack/validate/golangci-lint.yml

Looks like gogo/protobuf was updated in gogo/protobuf@9aee220 to match the correct format, but I found .pb.go files that were generated with an older version, e.g.

moby/builder/remotecontext/tarsum.pb.go

Lines 1 to 3 in 7a7357d

// Code generated by protoc-gen-gogo.

// source: tarsum.proto

// DO NOT EDIT!

We should have makefile targets to regenerate those files

can we regenerate in this PR? Or is there another PR for this?

Have not yet had time to look at how they were generated, and what's needed. I prefer to do that in a separate PR as it's less trivial (and we'll have to write scripts, and likely steps in CI to validate)

Opened #40176 as a tracking issue

thaJeztah · 2019-10-13T00:03:19Z

Unlikely to be related, but haven't seen this one before https://ci.docker.com/public/job/moby/job/PR-40077/2/execution/node/276/log/

22:25:12  === �[31mFailed�[0m
22:25:12  === �[31mFAIL�[0m: daemon TestRootMountCleanup/root_is_a_private_mountpoint (0.04s)
22:25:12      --- FAIL: TestRootMountCleanup/root_is_a_private_mountpoint (0.04s)
22:25:12          daemon_linux_test.go:228: assertion failed: error is not nil: path /tmp/TestRootMountCleanup563061781/daemon is mounted on /tmp/TestRootMountCleanup563061781 but it is not a shared mount
22:25:12  
22:25:12  === �[31mFAIL�[0m: daemon TestRootMountCleanup (0.16s)

kolyshkin · 2019-10-17T18:41:40Z

api/templates/server/operation.gotmpl

This is purely aesthetical, but can we please add a point or a comma before DO NOT EDIT?

As in

// Code generated by `swagger generate operation`. DO NOT EDIT.

kolyshkin · 2019-10-17T18:42:27Z

api/types/container/container_changes.go

kolyshkin

add a nit

thaJeztah · 2019-10-17T22:43:33Z

@kolyshkin updated

thaJeztah · 2019-10-24T01:53:52Z

@kolyshkin @cpuguy83 ptal

cpuguy83

LGTM

thaJeztah · 2019-11-05T19:32:00Z

let me rebase this one to get CI kicked again (failures should not be related to this change)

As described in https://golang.org/s/generatedcode, Go has a formalized format that should be used to indicate that a file is generated. Matching that format helps linters to skip generated files; From https://golang.org/s/generatedcode (golang/go#13560 (comment)); > Generated files are marked by a line of text that matches the regular expression, in Go syntax: > > ^// Code generated .* DO NOT EDIT\.$ > > The `.*` means the tool can put whatever folderol it wants in there, but the comment > must be a single line and must start with `Code generated` and end with `DO NOT EDIT.`, > with a period. > > The text may appear anywhere in the file. This patch updates the template used for our generated types to match that format. Signed-off-by: Sebastiaan van Stijn <[email protected]>

Signed-off-by: Sebastiaan van Stijn <[email protected]>

…rmat As described in https://golang.org/s/generatedcode, Go has a formalized format that should be used to indicate that a file is generated. Matching that format helps linters to skip generated files; From https://golang.org/s/generatedcode (golang/go#13560 (comment)); > Generated files are marked by a line of text that matches the regular expression, in Go syntax: > > ^// Code generated .* DO NOT EDIT\.$ > > The `.*` means the tool can put whatever folderol it wants in there, but the comment > must be a single line and must start with `Code generated` and end with `DO NOT EDIT.`, > with a period. > > The text may appear anywhere in the file. This patch updates the autogenerated code to match that format. Signed-off-by: Sebastiaan van Stijn <[email protected]>

kolyshkin

LGTM

full diff: moby/moby@a09e6e3...a9507c6 Includes: - moby/moby#40077 Update "auto-generate" comments to improve detection by linters - moby/moby#40143 registry: add a critical section to protect authTransport.modReq - moby/moby#40212 Move DefaultCapabilities() to caps package - moby/moby#40021 Use newer x/sys/windows SecurityAttributes struct (carry 40017) - carries moby/moby#40017 Use newer x/sys/windows SecurityAttributes struct - moby/moby#40135 pkg/system: make OSVersion an alias for hcsshim OSVersion - follow-up to moby/moby#39100 Use Microsoft/hcsshim constants and deprecate pkg/system.GetOsVersion() - moby/moby#40250 Bump hcsshim to b3f49c06ffaeef24d09c6c08ec8ec8425a0303e2 - moby/moby#40243 Use certs.d from XDG_CONFIG_HOME when in rootless mode - fixes moby/moby#40236 Docker rootless dies when unable to read /etc/docker/certs.d - moby/moby#40283 Fix possible runtime panic in Lgetxattr - moby/moby#40178 builder/remotecontext: small refactor - moby/moby#40179 builder/remotecontext: allow ssh:// for remote context URLs - fixes docker#2164 Docker build cannot resolve git context with html escapes - moby/moby#40302 client.ImagePush(): default to ":latest" instead of "all tags" - relates to docker#2214 [proposal] change "docker push" behavior to default to ":latest" instead of "all tags" - relates to docker#2220 implement docker push `-a`/ `--all-tags` - moby/moby#40263 Normalize comment formatting - moby/moby#40238 Allow client consumers like traefik to compile on illumos - moby/moby#40108 bump google.golang.org/grpc v1.23.1 - moby/moby#40312 update vendor golang.org/x/sys to 6d18c012aee9febd81bbf9806760c8c4480e870d - moby/moby#40247 pkg/system: deprecate constants in favor of golang.org/x/sys/windows - moby/moby#40246 pkg/system: minor cleanups and remove use of deprecated system.GetOSVersion() - moby/moby#40122 Update buildkit to containerd leases - vendor: update buildkit to leases support (4f4e03067523b2fc5ca2f17514a5e75ad63e02fb) - vendor: update containerd to acdcf13d5eaf0dfe0eaeabe7194a82535549bc2b - vendor: update runc to d736ef14f0288d6993a1845745d6756cfc9ddd5a (v1.0.0-rc9) Signed-off-by: Sebastiaan van Stijn <[email protected]>

full diff: moby/moby@a09e6e3...a9507c6 Includes: - moby/moby#40077 Update "auto-generate" comments to improve detection by linters - moby/moby#40143 registry: add a critical section to protect authTransport.modReq - moby/moby#40212 Move DefaultCapabilities() to caps package - moby/moby#40021 Use newer x/sys/windows SecurityAttributes struct (carry 40017) - carries moby/moby#40017 Use newer x/sys/windows SecurityAttributes struct - moby/moby#40135 pkg/system: make OSVersion an alias for hcsshim OSVersion - follow-up to moby/moby#39100 Use Microsoft/hcsshim constants and deprecate pkg/system.GetOsVersion() - moby/moby#40250 Bump hcsshim to b3f49c06ffaeef24d09c6c08ec8ec8425a0303e2 - moby/moby#40243 Use certs.d from XDG_CONFIG_HOME when in rootless mode - fixes moby/moby#40236 Docker rootless dies when unable to read /etc/docker/certs.d - moby/moby#40283 Fix possible runtime panic in Lgetxattr - moby/moby#40178 builder/remotecontext: small refactor - moby/moby#40179 builder/remotecontext: allow ssh:// for remote context URLs - fixes docker/cli#2164 Docker build cannot resolve git context with html escapes - moby/moby#40302 client.ImagePush(): default to ":latest" instead of "all tags" - relates to docker/cli#2214 [proposal] change "docker push" behavior to default to ":latest" instead of "all tags" - relates to docker/cli#2220 implement docker push `-a`/ `--all-tags` - moby/moby#40263 Normalize comment formatting - moby/moby#40238 Allow client consumers like traefik to compile on illumos - moby/moby#40108 bump google.golang.org/grpc v1.23.1 - moby/moby#40312 update vendor golang.org/x/sys to 6d18c012aee9febd81bbf9806760c8c4480e870d - moby/moby#40247 pkg/system: deprecate constants in favor of golang.org/x/sys/windows - moby/moby#40246 pkg/system: minor cleanups and remove use of deprecated system.GetOSVersion() - moby/moby#40122 Update buildkit to containerd leases - vendor: update buildkit to leases support (4f4e03067523b2fc5ca2f17514a5e75ad63e02fb) - vendor: update containerd to acdcf13d5eaf0dfe0eaeabe7194a82535549bc2b - vendor: update runc to d736ef14f0288d6993a1845745d6756cfc9ddd5a (v1.0.0-rc9) Signed-off-by: Sebastiaan van Stijn <[email protected]> Upstream-commit: 627a4cf7ccd0b7e92c6798c73de4dd4efc43175c Component: cli

full diff: moby/moby@a09e6e3...a9507c6 Includes: - moby/moby#40077 Update "auto-generate" comments to improve detection by linters - moby/moby#40143 registry: add a critical section to protect authTransport.modReq - moby/moby#40212 Move DefaultCapabilities() to caps package - moby/moby#40021 Use newer x/sys/windows SecurityAttributes struct (carry 40017) - carries moby/moby#40017 Use newer x/sys/windows SecurityAttributes struct - moby/moby#40135 pkg/system: make OSVersion an alias for hcsshim OSVersion - follow-up to moby/moby#39100 Use Microsoft/hcsshim constants and deprecate pkg/system.GetOsVersion() - moby/moby#40250 Bump hcsshim to b3f49c06ffaeef24d09c6c08ec8ec8425a0303e2 - moby/moby#40243 Use certs.d from XDG_CONFIG_HOME when in rootless mode - fixes moby/moby#40236 Docker rootless dies when unable to read /etc/docker/certs.d - moby/moby#40283 Fix possible runtime panic in Lgetxattr - moby/moby#40178 builder/remotecontext: small refactor - moby/moby#40179 builder/remotecontext: allow ssh:// for remote context URLs - fixes docker#2164 Docker build cannot resolve git context with html escapes - moby/moby#40302 client.ImagePush(): default to ":latest" instead of "all tags" - relates to docker#2214 [proposal] change "docker push" behavior to default to ":latest" instead of "all tags" - relates to docker#2220 implement docker push `-a`/ `--all-tags` - moby/moby#40263 Normalize comment formatting - moby/moby#40238 Allow client consumers like traefik to compile on illumos - moby/moby#40108 bump google.golang.org/grpc v1.23.1 - moby/moby#40312 update vendor golang.org/x/sys to 6d18c012aee9febd81bbf9806760c8c4480e870d - moby/moby#40247 pkg/system: deprecate constants in favor of golang.org/x/sys/windows - moby/moby#40246 pkg/system: minor cleanups and remove use of deprecated system.GetOSVersion() - moby/moby#40122 Update buildkit to containerd leases - vendor: update buildkit to leases support (4f4e03067523b2fc5ca2f17514a5e75ad63e02fb) - vendor: update containerd to acdcf13d5eaf0dfe0eaeabe7194a82535549bc2b - vendor: update runc to d736ef14f0288d6993a1845745d6756cfc9ddd5a (v1.0.0-rc9) Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah added status/2-code-review area/testing process/cherry-pick labels Oct 12, 2019

thaJeztah requested a review from tianon as a code owner October 12, 2019 20:01

thaJeztah commented Oct 12, 2019

View reviewed changes

kolyshkin reviewed Oct 17, 2019

View reviewed changes

api/types/container/container_changes.go Outdated

Copy link

Contributor

kolyshkin Oct 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

kolyshkin requested changes Oct 17, 2019

View reviewed changes

thaJeztah force-pushed the fix_autogen_detection branch from 84bc219 to 132b436 Compare October 17, 2019 22:42

thaJeztah force-pushed the fix_autogen_detection branch from 132b436 to 8da9503 Compare October 21, 2019 21:54

cpuguy83 approved these changes Nov 5, 2019

View reviewed changes

thaJeztah added 3 commits November 5, 2019 11:32

api/types: re-generate with new template

c511db7

Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah force-pushed the fix_autogen_detection branch from 8da9503 to 6186e9f Compare November 5, 2019 19:32

kolyshkin approved these changes Nov 5, 2019

View reviewed changes

kolyshkin merged commit c36460c into moby:master Nov 5, 2019

thaJeztah deleted the fix_autogen_detection branch November 5, 2019 21:10

thaJeztah mentioned this pull request Nov 5, 2019

Add make target (and verify step) to regenerate protobufs #40176

Closed

thaJeztah mentioned this pull request Jan 7, 2020

vendor: bump docker and dependencies a9507c6f76627fdc092edc542d5a7ef4a6df5eec docker/cli#2242

Merged

thaJeztah added this to the 20.03.0 milestone Apr 2, 2020

thaJeztah removed the process/cherry-pick label Feb 18, 2022

	// Code generated by protoc-gen-gogo.
	// source: tarsum.proto
	// DO NOT EDIT!

Conversation

thaJeztah commented Oct 12, 2019

Uh oh!

thaJeztah Oct 12, 2019

Choose a reason for hiding this comment

Uh oh!

justincormack Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

thaJeztah Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

thaJeztah Nov 5, 2019

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Oct 13, 2019

Uh oh!

kolyshkin Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

kolyshkin Oct 17, 2019

Choose a reason for hiding this comment

Uh oh!

kolyshkin left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Oct 17, 2019

Uh oh!

thaJeztah commented Oct 24, 2019

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Nov 5, 2019

Uh oh!

kolyshkin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants