Add Go guidelines to developer docs

What does this MR do?

Adds our Go guidelines to the development doc.

Related issues

Closes #54494 (closed)

Author's checklist

• Apply the correct labels and milestone
• Crosslink the document from the higher-level index: added to doc/development/README.md
• Crosslink the document from other subject-related docs: added to doc/development/contributing/style_guides.md
• Feature moving tiers? Make sure the change is also reflected in features.yml
• Correctly apply the product badges and tiers
• Port the MR to EE (or backport from CE): always recommended, required when the ee-compat-check job fails: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/9462

Review checklist

• Your team's review (required)
• PM's review (recommended, but not a blocker) (development guidelines)
• Technical writer's review (required)
• Merge the EE-MR first, CE-MR afterwards

/label ~Documentation golang development guidelines

doc/development/go_guide/index.md

@@ -6,7 +6,7 @@ projects using the [Go language][go].
 ## Overview
 
 GitLab is built on top of [Ruby on Rails][rails], but we're also using [Go][go]
-for projects where it makes sense. [Go][go] is a very powerful languages, with
+for projects where it makes sense. [Go][go] is a very powerful language, with
 many advantages, and is best suited for projects with a lot of IO (disk/network
 access), http requests, parallel processing, etc. Since we have both [Ruby on
 Rails][rails] and [Go][go] at GitLab, we should evaluate carefully which of the
@@ -24,20 +24,18 @@ https://github.com/golang/go/wiki/CodeReviewComments
 
 Reviewers and maintainers should pay attention to:
 
-- defer functions (add examples here)
-- inject dependencies as parameters 
+- `defer` functions: Ensure presence when needed, and after `err` check.
+- inject dependencies as parameters
 - void structs when marshalling to JSON (generates `null` instead of `[]`)
 
-TODO: Give examples
-
 ### Security
 
 Security is our top-priority at GitLab. During code reviews, we must take care
 of possible security breaches in our code:
 
-- XSS using text/template
+- XSS when using text/template
 - CSRF Protection using Gorilla
-- Use a Go version without know vulnerabilities
+- Use a Go version without known vulnerabilities
 - Don't leak secret tokens
 - SQL injections
 
@@ -46,54 +44,53 @@ Remember to run
 your project, or at least the [gosec
 analyzer](https://gitlab.com/gitlab-org/security-products/analyzers/gosec).
 
-Web servers can take advantages of middlewares like https://github.com/unrolled/secure 
+Web servers can take advantages of middlewares like https://github.com/unrolled/secure
 
 ### Finding a reviewer
 
 Many of our projects are too small to have full-time maintainers. That's why we
-have a shared pool of Go reviewers at GitLab.  To find a reviewer, use the
+have a shared pool of Go reviewers at GitLab. To find a reviewer, use the
 [Engineering Projects](https://about.gitlab.com/handbook/engineering/projects/)
-page in the handbook.  "GitLab Community Edition (CE)" and "GitLab Community
-Edition (EE)" both have a "Go" section with 
+page in the handbook. "GitLab Community Edition (CE)" and "GitLab Community
+Edition (EE)" both have a "Go" section with its list of reviewers.
 
-To add yourself as a GitLab Go reviewer, add the gitlab-ce and/or ee `go`
-subproject to the
+To add yourself to this list, add the following to your profile in the
 [team.yml](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/team.yml)
 file.
 
-```yaml 
-projects: 
-  gitlab-ee: reviewer go 
+```yaml
+projects:
+  gitlab-ee: reviewer go
   gitlab-ce: reviewer go
 ```
 
 ## Code style and format
 
-
-TODO
-- Avoid global variables, even in packages
-- use `go fmt`
+- Avoid global variables, even in packages. Doing so will introduce side
+  effects if the package is included multiple times.
+- Use `go fmt` before committing
 
 ### Automatic linting
 
 All go projects should include these GitLab-CI jobs:
 
 ```yaml
-go lint: 
+go lint:
   image: golang:1.11
   script:
     - go get -u golang.org/x/lint/golint
     - golint -set_exit_status
 ```
 
-(TODO: Share templates once nested includes are supported, like:
-https://gitlab.com/gitlab-org/security-products/ci-templates/raw/master/includes-dev/analyzer.yml)
+Once [recursive includes](https://gitlab.com/gitlab-org/gitlab-ce/issues/56836)
+will be available, we will be able to share job templates like:
+https://gitlab.com/gitlab-org/security-products/ci-templates/raw/master/includes-dev/analyzer.yml
 
 ## Dependencies
 
-Dependencies should be kept to the minimum.  The introduction of a new
-dependency should be argued in the Merge Request. Both License Management
-and Dependency Scanning should be activated on all projects to ensure new
+Dependencies should be kept to the minimum. The introduction of a new
+dependency should be argued in the Merge Request. Both License Management and
+Dependency Scanning should be activated on all projects to ensure new
 dependencies security status and licence compatibility.
 
 ### Modules
@@ -103,23 +100,33 @@ Modules](https://github.com/golang/go/wiki/Modules)". It provides a way to
 define and lock dependencies for reproducible builds. It should be used
 whenever possible.
 
+There was a [bug](https://github.com/golang/go/issues/29278) on Modules
+checksums in go <1.11.4, so make sure to use at least this version to avoid
+`checksum mismatch` errors.
+
 ### ORM
 
 We don't use ORMs at GitLab (except ActiveRecord, in Ruby on Rails of course).
 Projects can be structured with services to avoid them.
-[PQ](https://github.com/lib/pq) should be enough to interact with PostgreSQL databases.
+[PQ](https://github.com/lib/pq) should be enough to interact with PostgreSQL
+databases.
 
 ### Migrations
 
-TODO: https://github.com/db-journey/journey?
+In the rare event of managing a hosted database, it's necessary to use a
+migration system like ActiveRecord is providing. One simple library can be
+used: https://github.com/db-journey/journey It was designed to be used in
+`postgres` containers, that can be deployed as long-running pods. New versions
+will deploy a new pod, migrating the data automatically.
 
 ## Testing
 
-We don't use any specific framework for testing, as the standard library
-provides already everything to get started. Some external libraries might be
-used if needed:
+We should not use any specific library or framework for testing, as the standard library
+provides already everything to get started. For example, some external dependencies might be
+worth considering in case we decide to use a specific library or framework:
+
 - https://github.com/stretchr/testify
-- ?
+- https://github.com/gavv/httpexpect
 
 Use [subtests](https://blog.golang.org/subtests) whenever possible to improve
 code readability and test output.
@@ -133,7 +140,7 @@ Benchmarks, to ensure performance consistency over time.
 
 Every Go program is launched from the command line.
 [cli](https://github.com/urfave/cli) is a convenient package to create command
-line apps.  It should be used whether the project is a daemon or a simple cli
+line apps. It should be used whether the project is a daemon or a simple cli
 tool. Flags can be mapped to [environment
 variables](https://github.com/urfave/cli#values-from-the-environment) directly,
 which documents and centralizes at the same time all the possible command line
@@ -146,11 +153,11 @@ in the code.
 
 The usage of a logging library is strongly recommended for daemons. Even though
 there is a `log` package in the standard library, we generally use
-[logrus](https://github.com/sirupsen/logrus).  Its plugin ("hooks") system
+[logrus](https://github.com/sirupsen/logrus). Its plugin ("hooks") system
 makes it a powerful logging library, with the ability to add notifiers and
 formatters at the logger level directly.
 
-### Tracing
+### Tracing and Correlation
 
 [LabKit](https://gitlab.com/gitlab-org/labkit) is a place to keep common
 libraries for Go services. Currently it's vendored into two projects:
@@ -164,7 +171,7 @@ functionality:
   for distributed tracing.
 
 This gives us a thin abstraction over underlying implementations that is
-consistent across Workhorse, Gitaly, and, in future, other Go servers.  For
+consistent across Workhorse, Gitaly, and, in future, other Go servers. For
 example, in the case of `gitlab.com/gitlab-org/labkit/tracing` we can switch
 from using Opentracing directly to using Zipkin or Gokit's own tracing wrapper
 without changes to the application code, while still keeping the same
@@ -175,7 +182,7 @@ variable).
 
 Since daemons are long-running applications, they should have mechanisms to
 manage cancellations, and avoid unnecessary resources consumption (which could
-lead to DDOS vulnerabilities).  Go Context should be used in functions that can
+lead to DDOS vulnerabilities). Go Context should be used in functions that can
 block, and passed as the first parameter:
 https://github.com/golang/go/wiki/CodeReviewComments#contexts
 
@@ -183,7 +190,7 @@ https://github.com/golang/go/wiki/CodeReviewComments#contexts
 ## Dockerfiles
 
 Every project should have a `Dockerfile` at the root of their repository, to
-build and run the project.  Since Go program are static binaries, they should
+build and run the project. Since Go program are static binaries, they should
 not require any external dependency, and shells in the final image are useless.
 We encourage [Multistage
 builds](https://docs.docker.com/develop/develop-images/multistage-build/):
@@ -199,4 +206,5 @@ it will display its help message (if `cli` has been used).
 
 [Return to Development documentation](../README.md)
 
-[rails]: http://rubyonrails.org/ [go]: https://golang.org
+[rails]: http://rubyonrails.org/
+[go]: https://golang.org