Workaround for gofmt change in Go 1.11#2436
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2436 +/- ##
======================================
Coverage 45% 45%
======================================
Files 92 92
Lines 9412 9412
======================================
Hits 4236 4236
Misses 4493 4493
Partials 683 683
Continue to review full report at Codecov.
|
Go 1.11 uses a different formatting for maps, and now aligns values; running `gofmt -s -w` on this file resulted in this diff; ```patch content/testsuite/testsuite.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/content/testsuite/testsuite.go b/content/testsuite/testsuite.go index 974c7cb..d9ae9dc160 100644 --- a/content/testsuite/testsuite.go +++ b/content/testsuite/testsuite.go @@ -365,8 +365,8 @@ func checkLabels(ctx context.Context, t *testing.T, cs content.Store) { rootTime := time.Now().UTC().Format(time.RFC3339) labels := map[string]string{ - "k1": "v1", - "k2": "v2", + "k1": "v1", + "k2": "v2", "containerd.io/gc.root": rootTime, } @@ -402,7 +402,7 @@ func checkLabels(ctx context.Context, t *testing.T, cs content.Store) { } info.Labels = map[string]string{ - "k1": "v1", + "k1": "v1", "containerd.io/gc.root": rootTime, } preUpdate = time.Now() ``` Adding a whitespace before the long key to make it format the same on both Go 1.11 and older versions of Go. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
|
Opened a ticket upstream; golang/go#26228 |
|
Ok, this was marked as a "won't fix", "documentation only" change golang/go#26228 (comment), so if we want to support multiple versions of Go, we need to make some changes to CI and/or the contributing guide;
|
|
Do we need to support multiple versions of go formatting? I recommend we only ci test for the go format for one release.. preferably the latest version we are using in ci verification. |
|
So, the tricky bit is that contributors may not be on the latest version, so whenever they modify a file (and likely |
|
Also see the follow-up discussion on golang/go#26228 (comment) |
|
AYUP.. we can force them to move up, or we can move make builds into a container with the right go/gofmt pair that we are ci testing against so developers don't have to move up, or we can remove gofmt diffs == nil as a ci requirement (and maybe make it a warning/info message only), or???. Sometimes there's a text workaround, but not always. |
|
Yes, I don't think there's a one-size-fits-all solution unfortunately (at this moment); this patch was the least-invasive solution I could come up with for this time, while other solutions can be figured out |
|
hmm.. one size fits all would probably enable one of a plurality of go version formats as proof that the developer did in fact use gofmt, before commit. Your fix is less invasive yes, but requires an ugly line in the middle which would normally be called out as a nit pls fix comment in review :) Maybe open an issue to track for the larger problem and merge this as a temp soln? |
|
I don’t see a good reason to support multiple go versions for formatting. Albeit, this pr is harmless. LGTM |
|
LGTM |
Go 1.11 uses a different formatting for maps, and now aligns values; running
gofmt -s -won this file resulted in this diff (see #2435);Adding a whitespace before the long key to make it format the same on both Go 1.11 and older versions of Go.