github.com/m3db/m3@v1.5.1-0.20231129193456-75a402aa583b/STYLEGUIDE.md (about) 1 # M3 Coding Styleguide 2 3 M3's umbrella coding style guide is Uber's [Go Style Guide][uber-guide]. This 4 document is maintained as a superset of that guide, capturing any substantive, 5 intended departures from Uber's coding style. Where possible, code should follow 6 these guidelines to ensure consistent style and coding practices across the 7 codebase. 8 9 Above all else, this guide is intended to be a point of reference rather than 10 a sacred text. We maintain a style guide as a pragmatic tool that can be used to 11 avoid common issues and normalize code across many authors and the Go community. 12 13 New code should follow the style guide by default, preferring guidelines 14 established here or in Uber's guide over any conflicting, pre-existing 15 precedents in the M3 codebase. Ultimately, the hope is that the codebase 16 incrementally moves closer to the style guide with each change. 17 18 Since the M3 monorepo predated this style guide, reviewers should not expect 19 contributors to make unrelated or unreasonable style-based changes as part of 20 pull requests. However, when changing code that could reasonably be updated 21 to follow the guide, we prefer that those changes adopt the guidelines to avoid 22 sustaining or increasing technical debt. See DEVELOPMENT.md for more detail on 23 changes involving style. 24 25 [uber-guide]: https://github.com/uber-go/guide/blob/master/style.md 26 27 ## Linting 28 29 Many guidelines are flagged by `go vet` or the other configured linters (see 30 [.golangci.yml][.golangci.yml]). Wherever possible, we prefer to use tooling to 31 enforce style to remove subjectivity or ambiguity. Linting is also a blocking 32 build for merging pull requests. 33 34 [.golangci.yml]: https://github.com/m3db/m3/blob/master/.golangci.yml 35 36 ## Template 37 38 When adding to this guide, use the following template: 39 40 ~~~ 41 ### Short sentence about the guideline. 42 43 Clearly (and succinctly) articulate the guideline and its rationale, including 44 any problematic counter-examples. Be intentional when using language like 45 "always", "never", etc, instead using words like "prefer" and "avoid" if the 46 guideline isn't a hard rule. If it makes sense, also include example code: 47 48 <table> 49 <thead><tr><th>Bad</th><th>Good</th></tr></thead> 50 <tbody> 51 <tr><td> 52 53 ```go 54 goodExample := false 55 ``` 56 57 </td><td> 58 59 ```go 60 goodExample := true 61 ``` 62 63 </td></tr> 64 <tr><td> 65 Description of bad code. 66 </td><td> 67 Description of good code. 68 </td></tr> 69 </tbody></table> 70 ~~~ 71 72 ## Guidelines 73 74 ### Export types carefully. 75 76 Types should only be exported if they must be used by multiple packages. This 77 applies also to adding new packages: a new package should only be added if it 78 will be imported by multiple packages. If a given type or package will only 79 initially be imported in one package, define those type(s) in that importing 80 package instead. 81 82 In general, it's harder to reduce surface area than it is to incrementally 83 increase surface area, and the former is a breaking change while the latter is 84 often not. 85 86 ### Treat flaky tests like consistent failures. 87 88 Flaky tests add noise to code health signals, reduce trust in tests to be 89 representative of code behavior. Worse, flaky tests can be either false positive 90 or false negative, making it especially unclear as to whether or not a given 91 test passing or failing is good or bad. All of these reduce overall velocity 92 and/or reliability. 93 94 All tests discovered to be flaky should be immediately result in either (a) the 95 test being skipped because it is unreliable, or (b) master being frozen until 96 the test is fixed and proven to no longer be flaky. 97 98 ### Do not expose experimental package types in non-experimental packages. 99 100 A package is only able to guarantee a level of maturity/stability that is the 101 lowest common denominator of all of its composing or transitively exported 102 types. Given a hypothetical scenario: 103 104 ```go 105 package foo 106 107 type Bar { 108 Baz xfoo.Baz 109 } 110 ``` 111 112 In this case, the stability of `foo.Bar` is purportedly guaranteed by package 113 `foo` being non-experimental, but since it transitively exposes `xfoo.Baz` as 114 part of `foo.Bar`, either (a) `xfoo.Baz` must implicitly adhere to versioning 115 compatibility guarantees or (b) `foo` can no longer be considered stable, 116 as any breaking change to `xfoo.Baz` will break `foo`. 117 118 This is spiritually similar to the 119 [Avoid Embedding Types In Public Structs][avoid-embedding-types] guidance, in 120 that it bleeds implementation and compatibility details in an inappropriate way. 121 122 This guidance also applies to any cases in which `internal` packages are used: 123 any `internal` type is essentially the same as an unexported type, meaning that 124 that type is only implicitly available to users. 125 126 [avoid-embedding-types]: https://github.com/uber-go/guide/blob/master/style.md#avoid-embedding-types-in-public-structs 127 128 <table> 129 <thead><tr><th>Bad</th><th>Good</th></tr></thead> 130 <tbody> 131 <tr><td> 132 133 ```go 134 type NewConnectionFn func( 135 channelName string, addr string, opts Options, 136 ) (xclose.SimpleCloser, rpc.TChanNode, error) 137 ``` 138 139 </td><td> 140 141 ```go 142 type NewConnectionFn func( 143 channelName string, addr string, opts Options, 144 ) (io.Closer, rpc.TChanNode, error) 145 146 // or 147 148 type SimpleCloser = func() 149 150 type NewConnectionFn func( 151 channelName string, addr string, opts Options, 152 ) (SimpleCloser, rpc.TChanNode, error) 153 ``` 154 155 </td></tr> 156 <tr><td> 157 158 `xclose.SimpleCloser` is part of `x/close`, an experimental package, but is 159 directly exposed as part of `src/dbnode/client.NewConnectionFn`. 160 161 </td><td> 162 163 The canonical `io.Closer` is used instead, or a type alias representing 164 `xclose.SimpleCloser` is used instead. Both options prevent leaking experimental 165 packages as part of non-experimental library APIs. 166 167 </td></tr> 168 </tbody></table>