mullvad/coding-guidelines

Guides and texts about how we develop, format and work with code at Mullvad

coding-guidelines's Issues

Bash - Flag constants as read-only

Global constants

In your coding guidelines:

coding-guidelines/bash.md

Lines 58 to 62 in 5c5931d

	* Constant variables use `SCREAMING_SNAKE_CASE` names.
	This means variables that are assigned once and then never modified, and that only depend on
	other constant variables. The exception to "never modified" is where multiple statements are
	used to compute the content of the "constant" (see `COMPILE_FLAGS` in example at the end).

May I suggest to also declare these as readonly for more strictness?

The above example would become:

readonly SCREAMING_SNAKE_CASE="something"

Your usual:

SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

would become:

readonly SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

Local/scoped constants

On a very similar note, for variables scoped to their function with local:

coding-guidelines/bash.md

Lines 94 to 104 in 5c5931d

	Use `local` for variables inside functions unless they need to be global for some reason
	(They most likely should not be). This stops execution of functions from polluting the global
	namespace. Do this both for constants and other variables.
	```bash
	function cowsay {
	local COW_PREFIX="The cow says"
	local message="$1"
	echo "$COW_PREFIX: $message"
	}
	```

For those that should be constants, additionally to having them written in uppercase you could also declare them as read-only with local -r:

function cowsay { 
    local -r COW_PREFIX="The cow says"
    local message="$1" 
    echo "$COW_PREFIX: $message" 
} 

Add variable naming section on expressing unit of the type

If a type does not include the unit of the value, the variable name has to. Otherwise it becomes hard to know what is what.

// The type expresses the unit. Best solution.
let interval: Duration = Duration::from_millis(1001);

// The variable expresses the unit. Good solution for when the type can't or should not express the unit.
let interval_ms: u64 = 1001;

// Neither expresses the unit. Very hard to read, understand and properly use:
let interval: u64 = 1001;

Add section about Rust tests. Format and naming

Module named tests (plural). Don't start test functions with test_ etc. Unit tests vs integration tests.