3 ms·

A change to gofmt like that is going to require a large formatting commit to conform. Presumably/ideally there are CIs that fail on invalid format, so it's not

A change to gofmt like that is going to require a large formatting commit to conform. Presumably/ideally there are CIs that fail on invalid format, so it's not like you can wait until you touch the file again. Not a big deal, but still worth noting.

mappu 8y ago
There was a change to gofmt in 1.10, too: https://golang.org/doc/go1.10#gofmt https://golang.org/doc/go1.10#gofmt """Note that these kinds of minor updates to gofmt are expected from time to time. In general, we recommend against building systems that check that source code matches the output of a specific version of gofmt. For example, a continuous integration test that fails if any code already checked into a repository is not “properly formatted” is inherently fragile and not recommended."""
- kodablah 8y ago
  > For example, a continuous integration test that fails if any code already checked into a repository is not “properly formatted” is inherently fragile and not recommended. As a frequent repo/commit spelunker, I'm gonna have to disagree here. It is better to have this changed all in one commit instead of tacking on that formatting change along with an unrelated change elsewhere in the file and doing that all over the place at separate times. What's inherently fragile (formatting wise) is making a change to a file that does not meet the current formatting requirements and then having it format on save/commit, messing up blames, code review diffs, etc.
  - iainmerrick 8y ago
    Yep -- formatting for style is great iff it’s done rigorously and consistently on every commit (which is what a presubmit check is good for). Ad-hoc reformatting mingled in with real changes plays havoc with change tracking. Ideally the format would never change at all. An occasional change-the-world update is a reasonable compromise, but it should be done as a single commit with no logic changes.
Merovius 8y ago
Ideally, CI won't fail on invalid format, but just politely apply the correct format. There's nothing more frustrating than being told to do a job a machine can trivially to better (like executing "gofmt").
- iainmerrick 8y ago
  Right, so, checks and balances. Server-side you have the check. Client-side you have a hook that runs gofmt when you commit, and in principle you’ll never hit the server-side check. Just in case something goes wrong -- and something somewhere always goes wrong -- you don’t want the server to silently rewrite stuff on your behalf.

mappu 8y ago

There was a change to gofmt in 1.10, too: https://golang.org/doc/go1.10#gofmt https://golang.org/doc/go1.10#gofmt """Note that these kinds of minor updates to gofmt are expected from time to time. In general, we recommend against building systems that check that source code matches the output of a specific version of gofmt. For example, a continuous integration test that fails if any code already checked into a repository is not “properly formatted” is inherently fragile and not recommended."""

kodablah 8y ago
> For example, a continuous integration test that fails if any code already checked into a repository is not “properly formatted” is inherently fragile and not recommended. As a frequent repo/commit spelunker, I'm gonna have to disagree here. It is better to have this changed all in one commit instead of tacking on that formatting change along with an unrelated change elsewhere in the file and doing that all over the place at separate times. What's inherently fragile (formatting wise) is making a change to a file that does not meet the current formatting requirements and then having it format on save/commit, messing up blames, code review diffs, etc.
- iainmerrick 8y ago
  Yep -- formatting for style is great iff it’s done rigorously and consistently on every commit (which is what a presubmit check is good for). Ad-hoc reformatting mingled in with real changes plays havoc with change tracking. Ideally the format would never change at all. An occasional change-the-world update is a reasonable compromise, but it should be done as a single commit with no logic changes.

Merovius 8y ago

Ideally, CI won't fail on invalid format, but just politely apply the correct format. There's nothing more frustrating than being told to do a job a machine can trivially to better (like executing "gofmt").

iainmerrick 8y ago
Right, so, checks and balances. Server-side you have the check. Client-side you have a hook that runs gofmt when you commit, and in principle you’ll never hit the server-side check. Just in case something goes wrong -- and something somewhere always goes wrong -- you don’t want the server to silently rewrite stuff on your behalf.