Pull request: all: imp HACKING

Merge in DNS/adguard-home from imp-hacking to master Squashed commit of the following: commit cce8b05 Author: Ainar Garipov <[email protected]> Date: Thu Mar 25 16:47:26 2021 +0300 all: imp HACKING
heyxkhoa · Mar 25, 2021 · 88d9246 · 88d9246
1 parent a7f9e01
commit 88d9246
Showing 1 changed file with 85 additions and 40 deletions.
diff --git a/HACKING.md b/HACKING.md
@@ -1,19 +1,23 @@
  #  AdGuard Home Developer Guidelines
 
-As of **March 2021**, this document is partially a work-in-progress, but
-should still be followed.  Some of the rules aren't enforced as thoroughly or
-remain broken in old code, but this is still the place to find out about what we
-**want** our code to look like.
+As of **March 2021**, following this document is obligatory for all new code.
+Some of the rules aren't enforced as thoroughly or remain broken in old code,
+but this is still the place to find out about what we **want** our code to look
+like and how to improve it.
 
 The rules are mostly sorted in the alphabetical order.
 
+
+
 ## Contents
 
  *  [Git](#git)
  *  [Go](#go)
-     *  [Code And Naming](#code-and-naming)
+     *  [Code](#code)
      *  [Commenting](#commenting)
      *  [Formatting](#formatting)
+     *  [Naming](#naming)
+     *  [Testing](#testing)
      *  [Recommended Reading](#recommended-reading)
  *  [Markdown](#markdown)
  *  [Shell Scripting](#shell-scripting)
@@ -23,6 +27,8 @@ The rules are mostly sorted in the alphabetical order.
 <!-- NOTE: Use the IDs that GitHub would generate in order for this to work both
 on GitHub and most other Markdown renderers. -->
 
+
+
 ##  <a id="git" href="#git">Git</a>
 
  *  Call your branches either `NNNN-fix-foo` (where `NNNN` is the ID of the
@@ -47,14 +53,22 @@ on GitHub and most other Markdown renderers. -->
     The only exceptions are direct mentions of identifiers from the source code
     and filenames like `HACKING.md`.
 
+
+
 ##  <a id="go" href="#go">Go</a>
 
 > Not Golang, not GO, not GOLANG, not GoLang. It is Go in natural language,
 > golang for others.
 
 — [@rakyll](https://twitter.com/rakyll/status/1229850223184269312)
 
- ###  <a id="code-and-naming" href="#code-and-naming">Code And Naming</a>
+ ###  <a id="code" href="#code">Code</a>
+
+ *  Always `recover` from panics in new goroutines.  Preferably in the very
+    first statement.  If all you want there is a log message, use
+    `agherr.LogPanic`.
+
+ *  Avoid `errors.New`, use `aghnet.Error` instead.
 
  *  Avoid `goto`.
 
@@ -70,6 +84,14 @@ on GitHub and most other Markdown renderers. -->
     }
     ```
 
+    Except when the check is done to then use the first character:
+
+    ```go
+    if len(s) > 0 {
+            c := s[0]
+    }
+    ```
+
  *  Constructors should validate their arguments and return meaningful errors.
     As a corollary, avoid lazy initialization.
 
@@ -99,10 +121,11 @@ on GitHub and most other Markdown renderers. -->
     )
     ```
 
- *  Don't use naked `return`s.
+ *  Don't use `fmt.Sprintf` where a more structured approach to string
+    conversion could be used.  For example, `net.JoinHostPort` or
+    `url.(*URL).String`.
 
- *  Don't use underscores in file and package names, unless they're build tags
-    or for tests.  This is to prevent accidental build errors with weird tags.
+ *  Don't use naked `return`s.
 
  *  Don't write non-test code with more than four (**4**) levels of indentation.
     Just like [Linus said], plus an additional level for an occasional error
@@ -114,46 +137,20 @@ on GitHub and most other Markdown renderers. -->
  *  Eschew external dependencies, including transitive, unless
     absolutely necessary.
 
- *  Name benchmarks and tests using the same convention as examples.  For
-    example:
-
-    ```go
-    func TestFunction(t *testing.T) { /* … */ }
-    func TestFunction_suffix(t *testing.T) { /* … */ }
-    func TestType_Method(t *testing.T) { /* … */ }
-    func TestType_Method_suffix(t *testing.T) { /* … */ }
-    ```
-
- *  Name parameters in interface definitions:
-
-   ```go
-   type Frobulator interface {
-           Frobulate(f Foo, b Bar) (r Result, err error)
-   }
-   ```
-
- *  Name the deferred errors (e.g. when closing something) `derr`.
+ *  Minimize scope of variables as much as possible.
 
  *  No shadowing, since it can often lead to subtle bugs, especially with
     errors.
 
  *  Prefer constants to variables where possible.  Reduce global variables.  Use
     [constant errors] instead of `errors.New`.
 
+ *  Prefer to use named functions for goroutines.
+
  *  Program code lines should not be longer than one hundred (**100**) columns.
     For comments, see the text section below.
 
- *  Unused arguments in anonymous functions must be called `_`:
-
-    ```go
-    v.onSuccess = func(_ int, msg string) {
-            // …
-    }
-    ```
-
- *  Use linters.
-
- *  Use named returns to improve readability of function signatures.
+ *  Use linters.  `make go-lint`.
 
  *  Write logs and error messages in lowercase only to make it easier to `grep`
     logs and error messages without using the `-i` flag.
@@ -211,6 +208,49 @@ on GitHub and most other Markdown renderers. -->
     }}
     ```
 
+ ###  <a id="naming" href="#naming">Naming</a>
+
+ *  Don't use underscores in file and package names, unless they're build tags
+    or for tests.  This is to prevent accidental build errors with weird tags.
+
+ *  Name benchmarks and tests using the same convention as examples.  For
+    example:
+
+    ```go
+    func TestFunction(t *testing.T) { /* … */ }
+    func TestFunction_suffix(t *testing.T) { /* … */ }
+    func TestType_Method(t *testing.T) { /* … */ }
+    func TestType_Method_suffix(t *testing.T) { /* … */ }
+    ```
+
+ *  Name parameters in interface definitions:
+
+    ```go
+    type Frobulator interface {
+            Frobulate(f Foo, b Bar) (r Result, err error)
+    }
+    ```
+
+ *  Name the deferred errors (e.g. when closing something) `derr`.
+
+ *  Unused arguments in anonymous functions must be called `_`:
+
+    ```go
+    v.onSuccess = func(_ int, msg string) {
+            // …
+    }
+    ```
+
+ *  Use named returns to improve readability of function signatures.
+
+ ###  <a id="testing" href="#testing">Testing</a>
+
+ *  Use `assert.NoError` and `require.NoError` instead of `assert.Nil` and
+    `require.Nil` on errors.
+
+ *  Use functions like `require.Foo` instead of `assert.Foo` when the test
+    cannot continue if the condition is false.
+
  ###  <a id="recommended-reading" href="#recommended-reading">Recommended Reading</a>
 
  *  <https://github.com/golang/go/wiki/CodeReviewComments>.
@@ -223,6 +263,8 @@ on GitHub and most other Markdown renderers. -->
 [Linus said]:               https://www.kernel.org/doc/html/v4.17/process/coding-style.html#indentation
 [Text, Including Comments]: #text-including-comments
 
+
+
 ##  <a id="markdown" href="#markdown">Markdown</a>
 
  *  **TODO(a.garipov):** Define more Markdown conventions.
@@ -234,6 +276,8 @@ on GitHub and most other Markdown renderers. -->
  *  Use either link references or link destinations only.  Put all link
     reference definitions at the end of the second-level block.
 
+
+
 ##  <a id="shell-scripting" href="#shell-scripting">Shell Scripting</a>
 
  *  Avoid bashisms and GNUisms, prefer POSIX features only.
@@ -336,6 +380,7 @@ on GitHub and most other Markdown renderers. -->
     escaping is required or there are single quotes inside the string (e.g. for
     GitHub Actions).
 
- *  Use `>` for multiline strings, unless you need to keep the line breaks.
+ *  Use `>` for multiline strings, unless you need to keep the line breaks.  Use
+    `|` for multiline strings when you do.
 
 [NO-rway Law]: https://news.ycombinator.com/item?id=17359376