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.