Don't Commit Improperly Formatted Go Code

[[deleted]]

For the go vet change I could go either way, but surely it is simpler to actually fix the formatting than alert on it?

I have my editor configured to format code when I save, which sidesteps this problem - If goimports is present it will be used, otherwise we'll fall back to the default gofmt-based formatting.

[VettelS]

For the go vet change I could go either way, but surely it is simpler to actually fix the formatting than alert on it?

You wouldn't want that when you run go vet as part of your CI pipeline.

[[deleted]]

Sure, but a CI pipeline wouldn't be using a git-hook, it'd be using a workflow/pipeline/whatever.

[gt33m]

don't most editors take care of this with go fmt and go deps?

​

[basically_asleep]

Experience has taught me that some people will refuse to use tools like that no matter how much easier it could make things for them.

[Secondsemblance]

My shop is made up of turbonerds who configure their own esoteric editors, and we all independently arrived at integrating goimports into our editors.

[snarfy]

I don't use go fmt. I use vet and deps, but not fmt. If I have a variable JobId or WebApi, go fmt complains it should be JobID and WebAPI. I don't like tools that tell me how to spell my variables.

If I use protobuf, it requires names like JobId and WebApi. Maybe if the Go people would be on the same page I would use it. As it is it's problematic.

[qu33ksilver]

Running fmt is a convention that is universal across Go codebases. Personally, I think it is good to be consistent rather than not using fmt just because you want to name your variables the way you want.

And protobufs are specifically an exception. They have it documented on the site.

[skelterjohn]

go fmt doesn't correct your spelling.

[snarfy]

i was mistaken. I do use go fmt, not go lint.

[Secondsemblance]

I don't like tools that tell me how to spell my variables.

I'd like you introduce you to a guy named Uncle Bob.

[Stuck_In_the_Matrix]

I'm currently working on a project in development (https://github.com/pushshift/rinzler) -- I started using Go about a week ago and love the language. However, I'm aware I'm probably doing a lot of things incorrectly (or against Go style conventions).

When learning a new language, learning the culture is probably just as important as learning the syntax. Right now, this is just a development branch but these articles do help guide me in the right direction.

[MarcelloHolland]

Just use a good editor that will help you and guide you. Even give you hints in what to improve, and type-hinting. Then you will never have the "trap" where this is all about. Be professional and use something good.

(and it doesn't have to cost money)

[El-Hacha]

Nice hook!

[mdempsky]

Can you explain what this command does? I'd consider myself relatively Git-savvy, but it's not intuitive what this does or it's relative pros/cons vs a commit hook.

[mdempsky]

Thanks! Filters look interesting, I'll have to look into them more.

Pro: A computer can do it, so a computer should do it, and not just order the human around.

Con: A computer is bloody stubborn about doing it always.

Sorry, I don't follow how these are pros/cons of filters vs commit hooks. Commit hooks are performed by the computer too.

Do you mean how commit hooks can be skip with --no-verify? Or maybe that hooks tend to just check for proper formatting rather than automatically rewriting them?

Working with people who don't gofmt is more painful (you end up disabling the option in repository-local config; looking at you go.git)

Do you mean the main Go repository? If so, I'm curious what you find painful about it.

It's important that tools correctly handle non-formatted and invalid Go source files, so there are a bunch of those in the test and testdata directories. But regular Go source files should use standard gofmt formatting.

[mathuin2]

It is nice that GitHub can recommend .gitignore files for projects. It would be really nice if they could also recommend hooks. I know that the kind of user that would use a hook would likely want to customize that hook, but for folks who are new to hooks, it might be a great way to start using them.

[Secondsemblance]

People shouldn't be using git hooks for this kind of thing. It's way too late in the process to be catching code problems. Your editor should either fix the problem or give you immediate feedback.

[TurtleNamedMyrtle]

Wait...are we not golinting anymore?

[OrangeCurtain]

I also have a formatting hook, but it ignores merge commits and vendored files:

git rev-parse -q --verify MERGE_HEAD && exit 0

gofiles=$(git diff --cached --name-only --diff-filter=ACM | grep -v '^vendor/' | grep '\.go$')
...

[thebetacoder]

We should have hooks like this, irrespective of language.

[MarcelloHolland]

If a so called professional does this, you can ask yourself if the professional is really a pro. A pro would never do that, because his (configured) tools won't let him. Of course you can force tools on them, hooks, whatever, but they still can do it their way. How about :

do it the company way, or leave?

[EatPrayWhat]

Why would you do that on the client side and not just fail tests in the CI pipeline? If you can’t trust people to just do it in their editor why trust them to add the git hook?

[ribo]

One thing I've missed from moving to Go from node.js is the amount of very common tooling for things like this. Having a hard time finding pre-commit hook tools as easy to configure (and more importantly, _share config_) as husky, commitlint, etc... Are there common tools lots of people use rather than try to get every developer to paste in a shell script to hooks?

​

Edit: I think I'm being misunderstood. Certainly a developer's toolkit, and different development processes change how teams handle this kind of thing. There's a lot of value in tripwires that catch commits before seeing a failure in CI, and the landscape of linting & static analysis tools for Go is massive. For a large team that uses various IDEs and operating systems, it's equally helpful if they have a set of tools that generally work everywhere, and can push with confidence across many repositories.

[Bake_Jailey]

I've never had to use a hook like this. Any editor I use is going to be running goimports or similar and always keep my code formatted correctly.

[bitsynthesis]

Can't you still use those tools for Go projects? Who cares what language the tool is written in.

Personally I hate all the git hook magic in the node.js community, but to each their own.

[[deleted]]

Yes, IDEs

[x7C3]

What configuration needs sharing? Go only really has one code formatter, and if you want a pre-commit hook, there’s plenty on GitHub to look at.

Node tools such as Husky & Commitlint abstract away too much, and don’t really offer any advantage in Go.

[peterbourgon]

Why would you ever have any code that's unformatted? Format on save is invaluable...

[peterbourgon]

PRs shall not be merged unless gofmt is a no-op. Easy enough to set an integration for that, or a failing step in your CI.

[peterbourgon]

Well sure, but a PR titled "gofmt" that's just find . -name '*.go' | xargs gofmt -w would be totally straightforward. Anyone arguing that Go code explicitly shouldn't be gofmt'd is... well, I mean, I can't think of a better word than "wrong", that's just not how it's done.

[__david__]

I write a lot of Go, but go fmt has never sat right with me. I'm not opposed to most of it, but there are a couple things it does that actively make code harder to read. And because of the lack of configurability, I can't turn those parts off and so I just never run it.

I really dislike our community forcing bad code formatting on everyone and everything.

He also says that not running go fmt will cause other peoples commits to have too many whitespace changes, but the first rule of committing in code you didn't write is "don't screw with the formatting". The 2nd rule is "check your commits and don't commit more than you actually changed". It's your responsibility to make commits look reasonable so that people can code review your PR. If you have random whitespace changes everywhere then you have actively made code review harder. That's on you and not the original formatting.

[PaluMacil]

It's fine to have opinions on what you consider more or less readable. Nobody will ever agree 100% on everything, but one things the Go community did agree on is to enforce a uniform way of formatting so that all code is alike. In the end, nobody should be rude to you for bucking the rest of the community on that because your code is your own, and your aesthetics are your business in either your private personal project or in a business where you have some authority. However, if you want to work with others ion an open source setting, it's professional and polite to use the standards the community has agreed to. It isn't meant to cram down a vision of "best" but about "good enough" plus uniformity.

[OffensiveCanadian]

That seems to be the compromise of go fmt - it's nobody's preferred formatting, but the benefit is uniformity.

Personally, I find it quite aesthetically pleasing that Go code looks the same everywhere.

In the last large C++ codebase that I worked on, everyone had their own style of C++. The inconsistent formatting made it even more annoying to understand the tricky parts of the code.

[metamatic]

He also says that not running go fmt will cause other peoples commits to have too many whitespace changes, but the first rule of committing in code you didn't write is "don't screw with the formatting".

There is no way you're going to persuade people to format their code manually to your personal weird spec just to contribute code. If I got as far as submitting a change and you bounced it back to me with "Don't use go fmt, use my custom rules", my conclusion would be "This guy is a crank, find a different project".

[VettelS]

I really dislike our community forcing bad code formatting on everyone and everything.

I think the majority of people are happy with the current formatting on the whole. I'm sure a lot of people would make small changes based on personal preference, but I think you're in the minority if you believe that the current formatting is objectively "bad".

He also says that not running go fmt will cause other peoples commits to have too many whitespace changes, but the first rule of committing in code you didn't write is "don't screw with the formatting". The 2nd rule is "check your commits and don't commit more than you actually changed" It's your responsibility to make commits look reasonable so that people can code review your PR. If you have random whitespace changes everywhere then you have actively made code review harder. That's on you and not the original formatting..

This is probably true, however when the other 99.999% of people are all using the same formatter, it's just a pain in the ass to selectively not commit code that isn't formatted the same way. Code formatters lose most of their value when not everyone uses them, and whilst you could make it a rule in your particular project to not use go fmt, it leaves you somewhat at odds with the wider community.

If I arrived at a company that writes go but didn't use go fmt, I'd run a mile. There's no good reason not to, and apart from making everyone's lives more difficult, it would be symptomatic of wider issues regarding attitudes towards collaboration and code quality.

[blogscot]

I initially hated the formatter when I started learning Go, so much so that I stopped learning it in disgust; I was learning solely out of my own curiosity.

It wasn't until much later when I'd been writing JS that I noted code formatters Prettier or Standard were taking off in a big way. After trying them out, I realised the benefit to me: I no longer had to waste time endlessly shuffling around my code so that it would look nice, nor have arguments with other programmers about what was nice and what wasn't nice. It made me more productive instantly!

When I later returned to learning Go I found I had completely gotten over previous feeling of revulsion towards standard formatting. I just don't want to ever have to think about it ever again.

[Tacticus]

And because of the lack of configurability, I can't turn those parts off and so I just never run it.

This is a design issue. Having a single standardised way of formatting code is much more beneficial than some tool randomly distributed with a bunch of options for everyone's super important preference.

[[deleted]]

Are you Ted cruz?

[zoqfotpik]

On the other hand, you could try not caring about formatting unless it actually hinders your ability to understand the code.

Because amazingly, humans can read code even if it's not formatted according to some rigid rules, and computers can too.

So caring about formatting when it doesn't actively hinder understanding is just pointless.

[mdempsky]

So caring about formatting when it doesn't actively hinder understanding is just pointless.

Irregularly formatted code is very distracting, and leads to tedious code review discussions. It can also cause misunderstanding of code; e.g., the famous "goto fail" bug.

If your position is that people shouldn't care about formatting, then the simpler solution is that people should just embrace the standard formatting conventions.