Re: [Libguestfs] [libnbd PATCH 5/7] golang: Use 'gofmt' style recommendations on manual files

On Thu, Jul 27, 2023 at 01:52:00PM +0200, Laszlo Ersek wrote: > On 7/26/23 19:50, Eric Blake wrote: > > I ran: > > gofmt -s -w $(git ls-files -- '**/*.go') > > > > then touched up a few comments in test 590 where it mis-interpreted > > our intentions of having a single sentence that occupies more than 80 > > columns. > > > > Most of the changes are whitespace fixes (consistent use of TAB, > > altering the whitespace around operators), using preferred ordering > > for imports, and eliding excess syntax (unneeded () or ;). > > > > This patch only adjusts files directly in git control. Later patches > > will tackle (most) Go style problems in the generated files, then > > automate a way to ensure we don't regress. > > > > Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; > > --- > > +++ b/golang/libnbd_620_stats_test.go > > @@ -124,16 +124,16 @@ func Test620Stats(t *testing.T) { > > t.Fatalf("%s", err) > > } > > > > - if bs2 != bs1 + 28 { > > + if bs2 != bs1+28 { > > t.Fatalf("unexpected value for bs2") > > } > > - if cr3 != cr2 + slop { > > + if cr3 != cr2+slop { > > t.Fatalf("unexpected value for cr3") > > } > > } > > I've done nearly nothing in Go, and generally the updates look > justified... but the changes to "libnbd_620_stats_test.go" are hideous. > Who on Earth considers > > if cr3 != cr2+slop > > superior to > > if cr3 != cr2 + slop > > ??? > > *shudder* That's gofmt for you. I was surprised by it too. > > Question in passing: if we wrote > > if cr3 != (cr2 + slop) > > would gofmt contract that too to > > if cr3 != (cr2+slop) Equally surprising, gofmt (at least from golang-bin-1.20.6-1.fc38) does NOT remove the spaces from that format, nor does it complain that the () are spurious. Of course, future gofmt may change style again, but now we have a regression test in place to catch that. And we may be faced with decisions down the road on how to pacify cases where gofmt itself changes styles from one release of the language to the next (if that happens, that would be a strong argument that we should remove gofmt from a gating 'make check' test, and instead incorporate it as an optional 'make dist' step - if we reach a point where there is no longer "the" canonical formatting, we shouls still favor tarballs created with "a" canoncial format).

as well as amending the commit message to mention the manual touchup needed to work around the (in our opinion, misguided) advice from Go.