An opinionated Go Styleguide

Hey, I just released a Go Styleguide, please let me know what you think! :slight_smile:

1 Like

The style guide is indeed about opinion, at least the parts that are not common sense (i.e., the parts I disagree with vs the ones I agree with ;)). I’ll give you some opinions of mine, worth what you paid for them…

  • “Add context to errors”. No disagreement. However both here and in the actual package docs I miss a recommendation on when to wrap errors. As far as I can tell it must be the first time you receive an error from someone who does not use pkg/errors, and otherwise not. This can in practice translate to “first time you get an error from a function not in your own packages” but does require care at each error handlng point.

  • “Avoid gopkg.in”. :thumbsup: One more good reason.

  • “Structured logging”. I’m actually not sold on this. I see the advantages for large scale deployments where you are not really “logging” but serving metrics and events into some central system. I guess I just don’t do those deployments.

  • “Use testify”. I think this is my first strong disagreement. I don’t want assertion packages in my code and I don’t see what it brings to the table over normal, well written, tests.

  • “Avoid DeepEqual”. I see the point about a testString method that exposes the relevant attributes, that seems like a good idea. But apart from that it seems like you’re advocating assert.Equal instead which is probably the same thing?

  • “Don’t under-package”. I’m ambivalent. I hear the point about combining being easier than splitting, but I also think that once you’ve decided on a package structure there is inertia against changing it. That inertia can lead to odd API decisions with the purpose of just keeping the current package structure. I suspect it’s better to start with larger packages and split off parts that are clearly useful on their own. But I have no real data to back that up with.

  • “Handle signals”. I prefer to exit by crashing. Expecting to exit cleanly by signal seems fragile.

  • “Use import comment”. I have literally no idea what you’re saying here. import sub is not valid Go, afaik?

  • “Use Makefile if necessary”. :thumbsup: There is too much insistence, I think, on go install being the only thing you need. This is almost never the case for me. That said, I prefer a build.go over a Makefile - it’s easier to get to work on Windows, and the developers involved already understand the language.

The rest, which is the majority, is :thumbsup: as far as I’m concerned.

4 Likes

Thanks for the honest feedback!

It’s very hard to draw a line there, it depends on your application. I guess when you’re not sure if the error message is clear and understandable in your context, wrap them.

You can configure most (structured) loggers to print a console-friendly version, which makes sense in a terminal and still has the advantage over log.Sprintf to make values more clear/readable and changing the message won’t break searches (e.g. on Papertrail) for specific key/values.

I’ve written a lot of tests without testify, and complex cases will get big and hard to understand with all the if a != b { t.Errorf(c) }. Testify makes them easier to read and smaller (you save about 2 lines per assert) and provides a consistent error logging which otherwise requires discipline or guides.

I’m doing assert.Equal since I tried to be consistent in the style guide - it just compares the strings. This section actually may need some work to explain it only makes sense on really big structs or structs with children (e.g. trees).

It’s actually really hard to figure that out in reality since 20 packages with only one file each won’t make sense either. What I mean with this is: If you are unsure if to split a package or not, do it.

When you’re running a webserver in production you want to serve all open requests and stop accepting new ones (makes even more sense behind a lb). Or when writing to a database file you don’t want to risk corruption. Just exiting on SIGINT is dangerous.

This is a typo, I mean Use package comment package sub. Sorry for the confusion.

Makefile makes sense for generating data, making sure the buildflags are correct, running multiple tests/linters. A build.go is ok as well I guess, but more overhead due to the os.Exec() calls and also harder to read.

I hope this helps, I will update the styleguide to make some things more clear and fix the import/package typo.

Edit: I improved clarity and fixed the typo: fa09ec

1 Like

gopkg.in seem to have the most popular mongodb package. I couldn’t find an alternative for this, what do you suggest =/.
Very good reading material! Still going through it. Thanks alot

Some projects are tied to gopkg very tightly. If importing the GitHub repository with version tag does not work, just use the gopkg.in import, but be aware dep ensure -update won’t do anything there.

1 Like

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.