badguardhome/HACKING.md
Ainar Garipov fc9ddcf941 Pull request: all: client id support
Merge in DNS/adguard-home from 1383-client-id to master

Updates #1383.

Squashed commit of the following:

commit ebe2678bfa9bf651a2cb1e64499b38edcf19a7ad
Author: Ildar Kamalov <ik@adguard.com>
Date:   Wed Jan 27 17:51:59 2021 +0300

    - client: check if IP is valid

commit 0c330585a170ea149ee75e43dfa65211e057299c
Author: Ildar Kamalov <ik@adguard.com>
Date:   Wed Jan 27 17:07:50 2021 +0300

    - client: find clients by client_id

commit 71c9593ee35d996846f061e114b7867c3aa3c978
Merge: 9104f161 3e9edd9e
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Wed Jan 27 16:09:45 2021 +0300

    Merge branch 'master' into 1383-client-id

commit 9104f1615d2d462606c52017df25a422df872cea
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Wed Jan 27 13:28:50 2021 +0300

    dnsforward: imp tests

commit ed47f26e611ade625a2cc2c2f71a291b796bbf8f
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Wed Jan 27 12:39:52 2021 +0300

    dnsforward: fix address

commit 98b222ba69a5d265f620c180c960d01c84a1fb3b
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Jan 26 19:50:31 2021 +0300

    home: imp code

commit 4f3966548a2d8437d0b68207dd108dd1a6cb7d20
Merge: 199fdc05 c215b820
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Jan 26 19:45:13 2021 +0300

    Merge branch 'master' into 1383-client-id

commit 199fdc056f8a8be5500584f3aaee32865188aedc
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Jan 26 19:20:37 2021 +0300

    all: imp tests, logging, etc

commit 35ff14f4d534251aecb2ea60baba225f3eed8a3e
Author: Ildar Kamalov <ik@adguard.com>
Date:   Tue Jan 26 18:55:19 2021 +0300

    + client: remove block button from clients with client_id

commit 32991a0b4c56583a02fb5e00bba95d96000bce20
Author: Ildar Kamalov <ik@adguard.com>
Date:   Tue Jan 26 18:54:25 2021 +0300

    + client: add requests count for client_id

commit 2d68df4d2eac4a296d7469923e601dad4575c1a1
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Jan 26 15:49:50 2021 +0300

    stats: handle client ids

commit 4e14ab3590328f93a8cd6e9cbe1665baf74f220b
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Jan 26 13:45:25 2021 +0300

    openapi: fix example

commit ca9cf3f744fe197cace2c28ddc5bc68f71dad1f3
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Jan 26 13:37:10 2021 +0300

    openapi: improve clients find api docs

commit f79876e550c424558b704bc316a4cd04f25db011
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Jan 26 13:18:52 2021 +0300

    home: accept ids in clients find

commit 5b72595122aa0bd64debadfd753ed8a0e0840629
Merge: 607e241f abf8f65f
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Jan 25 18:34:56 2021 +0300

    Merge branch 'master' into 1383-client-id

commit 607e241f1c339dd6397218f70b8301e3de6a1ee0
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Jan 25 18:30:39 2021 +0300

    dnsforward: fix quic

commit f046352fef93e46234c2bbe8ae316d21034260e5
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Jan 25 16:53:09 2021 +0300

    all: remove wildcard requirement

commit 3b679489bae82c54177372be453fe184d8f0bab6
Author: Andrey Meshkov <am@adguard.com>
Date:   Mon Jan 25 16:02:28 2021 +0300

    workDir now supports symlinks

commit 0647ab4f113de2223f6949df001f42ecab05c995
Author: Ildar Kamalov <ik@adguard.com>
Date:   Mon Jan 25 14:59:46 2021 +0300

    - client: remove wildcard from domain validation

commit b1aec04a4ecadc9d65648ed6d284188fecce01c3
Author: Ildar Kamalov <ik@adguard.com>
Date:   Mon Jan 25 14:55:39 2021 +0300

    + client: add form to download mobileconfig

... and 12 more commits
2021-01-27 18:32:13 +03:00

8.0 KiB

AdGuard Home Developer Guidelines

As of December 2020, 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.

The rules are mostly sorted in the alphabetical order.

Git

  • Call your branches either NNNN-fix-foo (where NNNN is the ID of the GitHub issue you worked on in this branch) or just fix-foo if there was no GitHub issue.

  • Follow the commit message header format:

    pkg: fix the network error logging issue
    

    Where pkg is the directory or Go package (without the internal/ part) where most changes took place. If there are several such packages, or the change is top-level only, write all.

  • Keep your commit messages, including headers, to eighty (80) columns.

  • Only use lowercase letters in your commit message headers. The rest of the message should follow the plain text conventions below.

    The only exceptions are direct mentions of identifiers from the source code and filenames like HACKING.md.

Go

Not Golang, not GO, not GOLANG, not GoLang. It is Go in natural language, golang for others.

@rakyll

Code And Naming

  • Avoid goto.

  • Avoid init and use explicit initialization functions instead.

  • Avoid new, especially with structs.

  • Check against empty strings like this:

    if s == "" {
            // …
    }
    
  • Constructors should validate their arguments and return meaningful errors. As a corollary, avoid lazy initialization.

  • Don't use naked returns.

  • 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 write non-test code with more than four (4) levels of indentation. Just like Linus said, plus an additional level for an occasional error check or struct initialization.

    The exception proving the rule is the table-driven test code, where an additional level of indentation is allowed.

  • Eschew external dependencies, including transitive, unless absolutely necessary.

  • Name benchmarks and tests using the same convention as examples. For example:

    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:

type Frobulator interface {
        Frobulate(f Foo, b Bar) (r Result, err error)
}
  • Name the deferred errors (e.g. when closing something) cerr.

  • 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.

  • Unused arguments in anonymous functions must be called _:

    v.onSuccess = func(_ int, msg string) {
            // …
    }
    
  • Use linters.

  • Use named returns to improve readability of function signatures.

  • Write logs and error messages in lowercase only to make it easier to grep logs and error messages without using the -i flag.

Commenting

  • See also the Text, Including Comments section below.

  • Document everything, including unexported top-level identifiers, to build a habit of writing documentation.

  • Don't put identifiers into any kind of quotes.

  • Put comments above the documented entity, not to the side, to improve readability.

  • When a method implements an interface, start the doc comment with the standard template:

    // Foo implements the Fooer interface for *foo.
    func (f *foo) Foo() {
            // …
    }
    

    When the implemented interface is unexported:

    // Unwrap implements the hidden wrapper interface for *fooError.
    func (err *fooError) Unwrap() (unwrapped error) {
            // …
    }
    

Formatting

  • Add an empty line before break, continue, fallthrough, and return, unless it's the only statement in that block.

  • Use gofumpt --extra -s.

  • Write slices of struct like this:

    ts := []T{{
            Field: Value0,
            // …
    }, {
            Field: Value1,
            // …
    }, {
            Field: Value2,
            // …
    }}
    

Markdown

  • TODO(a.garipov): Define our Markdown conventions.

Shell Scripting

  • Avoid bashisms and GNUisms, prefer POSIX features only.

  • Prefer 'raw strings' to "double quoted strings" whenever possible.

  • Put spaces within $( cmd ), $(( expr )), and { cmd; }.

  • Put utility flags in the ASCII order and don't group them together. For example, ls -1 -A -q.

  • snake_case, not camelCase for variables. kebab-case for filenames.

  • UPPERCASE names for external exported variables, lowercase for local, unexported ones.

  • Use set -e -f -u and also set -x in verbose mode.

  • Use readonly liberally.

  • Use the "$var" form instead of the $var form, unless word splitting is required.

  • When concatenating, always use the form with curly braces to prevent accidental bad variable names. That is, "${var}_tmp.txt" and not "$var_tmp.txt". The latter will try to lookup variable var_tmp.

  • When concatenating, surround the whole string with quotes. That is, use this:

    dir="${TOP_DIR}/sub"
    

    And not this:

    # Bad!
    dir="${TOP_DIR}"/sub
    

Text, Including Comments

  • End sentences with appropriate punctuation.

  • Headers should be written with all initial letters capitalized, except for references to variable names that start with a lowercase letter.

  • Start sentences with a capital letter, unless the first word is a reference to a variable name that starts with a lowercase letter.

  • Text should wrap at eighty (80) columns to be more readable, to use a common standard, and to allow editing or diffing side-by-side without wrapping.

    The only exception are long hyperlinks.

  • Use U.S. English, as it is the most widely used variety of English in the code right now as well as generally.

  • Use double spacing between sentences to make sentence borders more clear.

  • Use the serial comma (a.k.a. Oxford comma) to improve comprehension, decrease ambiguity, and use a common standard.

  • Write todos like this:

    // TODO(usr1): Fix the frobulation issue.
    

    Or, if several people need to look at the code:

    // TODO(usr1, usr2): Fix the frobulation issue.
    

YAML

  • TODO(a.garipov): Define naming conventions for schema names in our OpenAPI YAML file. And just generally OpenAPI conventions.

  • TODO(a.garipov): Find a YAML formatter or write our own.

  • All strings, including keys, must be quoted. Reason: the NO-rway Law.

  • Indent with two (2) spaces. YAML documents can get pretty deeply-nested.

  • No extra indentation in multiline arrays:

    'values':
    - 'value-1'
    - 'value-2'
    - 'value-3'
    
  • Prefer single quotes for strings to prevent accidental escaping, unless 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.