Code styling guidelines

[moul]

Following this discussion -> #400 (comment).

We’ve multiple styles which are all valid golang and all passing the current linter set.

To make the code more consistent and easier to read, I suggest:

• discussing here and getting feedback, to select advanced guidelines.
• document them on a CONTRIBUTING.md file or somewhere else where it makes sense.
• then try to configure linters, editorconfig, etc to make the CI, our editor all aware of our preferences.

To bootstrap the discussion, is anyone aware of such discussion on other repos where we can get inspiration? I’m pretty sure this is a very common topic.

[piux2]

These are good starting points.

https://google.github.io/styleguide/go/guide
https://google.github.io/eng-practices/review/

[zivkovicmilos]

I've worked a lot with code linting and style guides. For Go, golangci-lint is really the front-runner in terms of features and ease of use (it simply bundles multiple other linters for you and execs them). You simply write up a .golangci.yaml, and the tooling takes care of the rest (locally, and in GitHub Actions).

Here is a list of linters I've had success with in the past:

    - whitespace # Tool for detection of leading and trailing whitespace
    - wsl # Forces you to use empty lines
    - wastedassign # Finds wasted assignment statements. This one is funky with Go 1.19
    - unconvert # Unnecessary type conversions
    - tparallel # Detects inappropriate usage of t.Parallel() method in your Go test codes
    - thelper # Detects golang test helpers without t.Helper() call and checks the consistency of test helpers
    - stylecheck # Stylecheck is a replacement for golint
    - prealloc # Finds slice declarations that could potentially be pre-allocated
    - predeclared # Finds code that shadows one of Go's predeclared identifiers
    - nolintlint # Ill-formed or insufficient nolint directives
    - nlreturn # Checks for a new line before return and branch statements to increase code clarity
    - misspell # Misspelled English words in comments
    - makezero # Finds slice declarations with non-zero initial length
    - lll # Long lines
    - importas # Enforces consistent import aliases
    - gosec # Security problems
    - gofmt # Whether the code was gofmt-ed
    - goimports # Unused imports
    - goconst # Repeated strings that could be replaced by a constant
    - forcetypeassert # Finds forced type assertions
    - dogsled # Checks assignments with too many blank identifiers (e.g. x, , , _, := f())
    - dupl # Code clone detection
    - errname # Checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error
    - errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13

You can find a full list of supported linters here, these ones I've listed are just some I've been using for a while now.

As for the code style guide, I almost exclusively follow Uber's Go style guide in my work, as I believe it's one of the most clean ones out there, and it keeps the code in check.

If you'd like, we can discuss which linters make sense for us, and then I can follow through with clearing up linting errors with a PR

[moul]

So, we'll do two things.

1. soon, update the CONTRIBUTING.md file to say that in addition to passing current linters, contributions should also follow the main style of the repo for consistency.
2. periodically, try to improve the linters configuration to enforce more consistency.

[zivkovicmilos]

@piux2 @moul

I've opened up a small PR to tackle updating the CONTRIBUTING.md file, to include more information, as well as some basic code style guidelines we've discussed here.

I'll open a new PR for the linter and linter workflows next.

[zivkovicmilos]

@moul @piux2

I've opened up a second PR to tackle the linting itself.

Please review the list of linters I've proposed so we can proceed with resolving present linting errors.

[moul]

Closing, but if anyone has advice, please open a new issue.