/ archive / Go_Code_Quality_Notes.md
Go_Code_Quality_Notes.md
 1  This document describes recommendations for writing quality code for
 2  Status.
 3  
 4  ## Background
 5  
 6    - Look at these documents for general Go coding standards and
 7      recommendations:
 8    - Common mistakes in Go (mostly for Go newcomers):
 9      <http://devs.cloudimmunity.com/gotchas-and-common-mistakes-in-go-golang/>
10    - Effective Go: <https://golang.org/doc/effective_go.html>
11    - Code review comments:
12      <https://github.com/golang/go/wiki/CodeReviewComments>
13    - Each link is an invaluable source of widely accepted practices.
14  
15  ## Recommendations
16  
17  1.  Be a good git-citizen, keep commit messages to the point, do `git
18      pull --rebase` where possible. Read
19      <https://www.derekgourlay.com/blog/git-when-to-merge-vs-when-to-rebasefor>
20      more details.
21  2.  When leaving TODO comments, don’t forget to specify your slack
22      nickname, for example:`// TODO(tiabc): Fix the underlying
23      processing.`This helps other people reach the appropriate person to
24      elaborate when needed.
25  3.  Use `testify` for testing, it’s more verbose than the default
26      testing library.
27  4.  Tests should test what you expect them to test and should NOT test
28      unrelated code. Lots of tests in Status needs refactoring.
29  5.  Tests are preferred to be in `Arrange-Act-Assert` format. It brings
30      a unified structure to them and creates eye catchers for better
31      navigation, for <example:This> may look like overhead but it brings
32      structure, forces you to write different tests for different checks
33      and proves fruitful in long-term.
34  
35  <!-- end list -->
36  
37  ```
38    func (s *JailTestSuite) TestParse() {
39        // Arrange.
40        require := s.Require()
41        extraCode := `
42        var _status_catalog = {
43            foo: 'bar'
44        };
45        `
46  
47        // Act.
48        response := s.jail.Parse("newChat", extraCode)
49  
50        // Assert.
51        expectedResponse := `{"result": {"foo":"bar"}}`
52        require.Equal(expectedResponse, response)
53    }
54  ```
55  
56  1.  Before sending a pull request:
57  
58  <!-- end list -->
59  
60    - Review your code yourself and make sure everything is up to your
61      code quality bar.
62    - Ensure the build is successful.
63    - Ensure tests pass.
64    - Ensure `go fmt`, `go lint` and `go vet` produce no warnings. If they
65      do, fix them or add ignore rules.
66    - Make sure you’re sending the pull requests from the same repository
67      if you have permissions to create branches.
68    - If you’re sending the pull request to ask for advice, explicitly
69      state it to the reviewer so that he or she review only what’s
70      necessary to answer the question.
71  
72  From @anna (QA):
73  
74  > btw, I also like Jan’s style in PR summary content for a bug fix, e.g.
75  > <https://github.com/status-im/status-react/pull/1509> , it explains
76  > the cause and the fix done. So, I can get more insights while
77  > verifying the bug. It might be not applicable in all fixes but I would
78  > appreciate if there is some notes on how bug is fixed. I will not look
79  > in the files changed for each PR and will not understand everything in
80  > the code, so some summary is helpful.