diff --git a/_includes/search.html b/_includes/search.html index 504d8cd7f..6a70a47d7 100644 --- a/_includes/search.html +++ b/_includes/search.html @@ -49,7 +49,7 @@
aws logo
aws logo
-
aws logo
+
aws logo
diff --git a/_layouts/post.html b/_layouts/post.html index ff877e1b2..d1b188647 100644 --- a/_layouts/post.html +++ b/_layouts/post.html @@ -32,11 +32,11 @@ / {{ page.title | replace:'-',' ' }} {% else %} / {{ crumb | replace:'-',' ' | capitalize }}/ {{ crumb | url_decode | replace:'-',' ' | capitalize }} {% endif %} {% endfor %} diff --git a/_posts/2020-11-09-golang-style-guide.adoc b/_posts/2020-11-09-golang-style-guide.adoc new file mode 100644 index 000000000..e8adaacc2 --- /dev/null +++ b/_posts/2020-11-09-golang-style-guide.adoc @@ -0,0 +1,266 @@ +--- +title: Go Style Guide +category: Style Guides +image: /assets/img/guides/go-logo-blue.png +excerpt: The style guide for the Go programming language that is used by all Gruntwork-owned code repositories that contain Go code. +tags: ["go", "golang", "code", "style"] +cloud: ["aws", "gcp"] +redirect_from: /static/guides/styleguides/golang-style-guide/ +--- +:page-type: guide +:page-layout: post + +:toc: +:toc-placement!: + +// GitHub specific settings. See https://gist.github.com/dcode/0cfbf2699a1fe9b46ff04c41721dda74 for details. +ifdef::env-github[] +:tip-caption: :bulb: +:note-caption: :information_source: +:important-caption: :heavy_exclamation_mark: +:caution-caption: :fire: +:warning-caption: :warning: +toc::[] +endif::[] + +== Intro +This is Gruntwork's style guide for the Go programming language. It aims to help us ensure that the code we write is +clear, readable, idiomatic Go code. The conventions detailed in this guide are our preferences and should be thought of +as guidelines rather than hard rules. + +== Starting point +We use the excellent https://golang.org/doc/effective_go.html[Effective Go] guide as the starting point for our style +guide. **Unless explicitly mentioned otherwise, we default to following the conventions outlined in Effective Go.** + +Another helpful resource is the https://github.com/golang/go/wiki/CodeReviewComments[CodeReviewComments] section of the +Go GitHub wiki page. + +Below you will find the list of conventions that differ from the above mentioned documents, either because they are +specific to Gruntwork, or because we consciously chose to go against what Effective Go and CodeReviewComments recommend. + +== Additional conventions +=== General +Before committing any Go code, run `go fmt`. We run this as part of the CI build, so this will prevent your build from failing. + +=== Comments +Every public function `Foo` should have a `//` comment of the style `Foo `, +where explanation says what `Foo` does, and more importantly, why it does that, what assumptions it makes, and other +aspects not obvious from the function name. + +=== Control structures +When possible, avoid nesting `if`/`else` statements. (Of course, a single, non-nested `if`/`else` is perfectly fine.) +Prefer multiple `return` statements over nested code. This makes the code more readable and avoids complexity. + +E.g: +[source,go] +---- +// Do this +if something { + doThis() + return this +} +if orOther { + return that +} +return theOther +---- + +[source,go] +---- +// Don't do this +if something { + doThis() +} else { + if orOther { + return that + } +} +---- + +=== Error handling +Prefer using the `errors` standard library package for handling single errors. For operations that can produce multiple +errors, leverage the https://github.com/gruntwork-io/terragrunt/blob/master/errors/multierror.go[`MultiError`] +package by https://github.com/gruntwork-io/terragrunt/blob/cb369119bf5c6f3031e914e8554ffe056dcf9e22/cli/hclfmt.go#L62[accumulating +all the errors into a single `MultiError` and returning that], rather than returning every error individually as it comes up. + +https://github.com/golang/go/wiki/CodeReviewComments#dont-panic[Don't panic] (_hat tip, Douglas Adams_). Any method that +can have an error should return that error as its final value. This should be passed up through each layer, which can +decide what to do with it, all the way up to the very entrypoint of the app (or `main`) if necessary. +You should almost NEVER use `panic`. + +Use custom error types. Create your own types that implement the `error` interface so that error messages are clear +and have well-defined types you can check against. For some examples of this, see e.g. the custom erros in the +https://github.com/gruntwork-io/terratest/blob/master/modules/aws/errors.go[aws] package of `terratest`. + +Include stack traces. In most of our code, we have to wrap errors with +https://github.com/gruntwork-io/gruntwork-cli/blob/master/errors/errors.go#L22[`errors.WithStackTrace(e)`] to add the stack trace. +Go annoyingly doesn't do this by default, but without it, sorting out an error can be very tricky. + +=== Pointer usage +Prefer using value type over pointers, unless necessary. Generally speaking, there are only a few cases where pointer +usage would be justified: + +1. You have very large structs containing lots of data. Instead of copying these around, passing pointers may be more + efficient. +2. You need to mutate a variable you passed to a function. Generally speaking, you should avoid this anyway (see the + section on immutability below), but sometimes it is necessary (think "rename" methods, etc). +3. You need to return `nil` as the default zero value (the default zero value for pointers is `nil`, as opposed to other data + types that have non-nil default zero values). + +=== Testing +In terms of testing, we don't necessarily aim for 100% test coverage. Rather, our goal is to have enough testing to give +us confidence that our code works and allow us to update it quickly. When adding tests, keep in mind these factors: + +1. The likelihood of bugs: some code is harder to get right than others, and merits correspondingly more tests. +2. The cost of bugs: some bugs are much more costly than others — e.g., bugs in payment systems or security functionality — + so it makes sense to invest more in testing there. +3. The cost of tests: some types of tests are cheap and easy to write and maintain, whereas others are very costly and brittle. + +Unless there's a reason not to, tests should run in parallel. This is done by including a call to `t.Parallel` in the test function. + +==== Setup and Teardown pattern + +In some cases you will want to write a group of tests that use a common resource, such as a Docker image or VPC. +In this case, you will want to setup the common resource once, run a bunch of tests, and then teardown the resource. +To achieve this, you can follow https://blog.golang.org/subtests[the subtest pattern] of Go. + +Always use https://dave.cheney.net/2019/05/07/prefer-table-driven-tests[table driven tests] where possible to make the +subtest routines maintainable. Briefly, this means that you group your test cases using a test struct that reflects +the unique parameters of the test cases. Then you can conveniently loop over the test cases in parallel, taking advantage +of uniformity and speed. + +Note that the subtest pattern has gotchas when running tests in parallel: + +- The main test function will not wait for the subtest to run if it uses `t.Parallel`. To avoid this, you need to wrap + the parallel subtests in a synchronous, blocking subtest. In the example below, the `group` subtest is synchronous + (no call to `t.Parallel`) and thus the main function will wait until that test finishes. The `group` test does not + finish until all the subtests it spawns are finished, even if they are non-blocking and parallel, and thus the + `tearDownVPC` call does not happen until all subtests are done. +- If you are using table driven tests, the range variable will be updated to the next iteration before it is used within + the subtest. That is, in the example below, if we did not have the `testCase := testCase` line in the range block, + the `testCase` reference used in the subtest after the `t.Parallel` call will correspond to the last `testCase` in the + `testCases` list. To avoid this, we create a new variable in the scope of the range block so that it does not get + updated during the loop. + +Example: + +[source,go] +---- +func TestECS(t *testing.T) { + t.Parallel() + + defer tearDownVPC() + deployVPC() + + // Wrap the parallel tests in a synchronous test group to + // ensure that the main test function (the one calling + // `tearDownVPC` and `deployVPC`) waits until all the + // subtests are done before running the deferred function. + t.Run("group", func(t *testing.T) { + for _, testCase := range testCases { + // To avoid the range variable from getting updated in the + // parallel tests, we bind a new name that is within the + // scope of the for block. + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + testCase.testCode() + }) + } + }) +} +---- + +=== Naming things +Prefer descriptive names over short ones. In particular, avoid one-letter variable names, other than in case of very well +known and widely understood conventions, such as `i` for `index` (e.g. in a loop). A more descriptive name helps with +code understanding and maintenance, at very little cost, given the auto-complete feature in most IDEs and editors. + +If a name contains an acronym, only capitalize the first letter of the acronym. E.g. use `someEksCluster` rather than +`someEKSCluster`. We go against the https://github.com/golang/go/wiki/CodeReviewComments#initialisms[recommendation] +here in order to follow the convention already in use by some third party packages we heavily rely on (e.g. `aws-sdk-go`). + +==== Constants +Since many languages use `ALL_CAPS` for constants, it is worth calling out explicitly that +https://golang.org/doc/effective_go.html#mixed-caps[Effective Go] recommends using `MixedCaps` for all names, including constants. +Therefore, `region` or `testRegion` for private constants and `Region` or `TestRegion` for public ones is preferred over +`REGION` or `TEST_REGION`. + +=== Functional programming practices + +==== Immutability +Prefer returning a new value rather than mutating an existing one. + +[source,go] +---- +// Don't do this +var result int = 0 + +func main() { + add(1, 1, &result) + fmt.Println(result) +} + +func add(a, b int, result *int) { + *result = a + b +} +---- + +[source,go] +---- +// Do this instead +func main() { + fmt.Println(add(1, 1)) +} + +func add(a, b int) int { + return a + b +} +---- + +==== Pure functions +Prefer functions that take all their inputs as function parameters, instead of reading those inputs via side effects +(e.g., reading from disk or the network or global vars), and whose entire behavior is to return values +(note: errors are values too!), rather than performing side effects (e.g. by writing to disk or the network or global +vars). Of course, you can't avoid side effects forever if you want your code to do something useful, but try to do as +much of your logic with pure functions as you can, try to pass everything around as explicit parameters and return +everything as explicit values, and centralize the side effecting code to a few isolated places. + +==== Composition +Build your code out of small, reusable, named functions, that you compose together. + + +[source,go] +---- +// Don't do this +func main() { + fmt.Println(mulOfSums(1, 1)) +} + +func mulOfSums(a, b int) int { + return (a + b) * (a + b) +} +---- + +[source,go] +---- +// Do this instead +func main() { + fmt.Println(mul(add(1, 1), add(1, 1))) +} + +func add(a, b int) int { + return a + b +} + +func mul(a, b int) int { + return a * b +} +---- + +=== Repo-specific conventions +==== terratest +Note the existence of methods in terratest which are suffixed with the letter `E`, e.g. +https://github.com/gruntwork-io/terratest/blob/master/modules/aws/account.go#L23[GetAccountIdE]. Methods that have the +suffix `E` return an error as the last return value; methods without `E` mark the test as failed +(e.g., via calling `t.Fail()`) instead of returning an error. diff --git a/assets/img/guides/go-logo-blue.png b/assets/img/guides/go-logo-blue.png new file mode 100644 index 000000000..d6d98c3bd Binary files /dev/null and b/assets/img/guides/go-logo-blue.png differ diff --git a/assets/js/search.js b/assets/js/search.js index 59e8210a6..2e1803db5 100644 --- a/assets/js/search.js +++ b/assets/js/search.js @@ -98,6 +98,16 @@ window.guideEntries; } + /** + * A naive function to slugify a string input and return a URL friendly output + * of that string. This implementation was sourced from: + * https://stackoverflow.com/a/1054862 + * @param {*} stringValue + */ + function naiveSlugify(stringValue) { + return stringValue.replace(/ /g,'-').replace(/[^\w-]+/g,'') + } + /** * A function to display or hide the category of search * @type {Function} @@ -105,7 +115,8 @@ function displayCategory(entry) { const categoryArr = $.merge($('.category-head'), $('.categories ul li')); categoryArr.each(function () { - const category = $(this).text().toLowerCase(); + const category = naiveSlugify($(this).text().toLowerCase()); + if (entry.category === category) { $(`.categories ul .${category}`).show(); $(`#${category}.category-head`).show(); diff --git a/pages/guides/_guides.html b/pages/guides/_guides.html index 4a870fa16..5ea6a74b5 100644 --- a/pages/guides/_guides.html +++ b/pages/guides/_guides.html @@ -5,7 +5,7 @@