Style guidelines

For conventions when contributing to the MediaWiki codebase, see mw:Manual:Coding conventions

Style guidelines are an important part of an organization; As a team, we should all have expectations of how code will be written and read so that we can all contribute with the lowest barrier to entry.

Consistency reduces complexity and bugs, which in turn keeps codebases accessible to all. Code is more often read than written, so it's important to keep everything readable.

Git

Commit messages

Git commits messages are an important part of a coherent evolution of a codebase. It will help future programmers understand the context of a change. Git is an essential tool for discovering not only where changes have occurred by why changes have occurred.

To help facilitate this:

• Commit subject must be 50 characters or fewer. The subject is to give an overview of your work. If you feel heavily restricted by this, consider that your commits are too broad and need to be more atomic.

• Body messages must wrap at 72 characters. Formatting is to be done at commit time for a reliable, readable message. Do not assume that tools will properly format for you.

Preparing a change request is preparing a patchset for inclusion in the main branch. This is your chance to tart up your code changes. Use git-rebase(1) liberally so that your changes are properly formed and well-documented. See https://git-rebase.io/ for a decent run-down of how and why this is important.

Puppet

See Puppet/Coding and style guidelines#Coding_Style

Shell

This style guide is based off of Google’s with a number of alterations.

Consider if your code will need to run in a constrained environment where only a basic POSIX shell will be available; in that case, stick to strict POSIX shell constructs and avoid bashisms (of note: all of wikimedia docker images include bash). Else, use bash features as long as they improve the readability and maintainability of the file. Of note, POSIX shell lacks the pipefail option, so consider using Bash by default when lots of pipes are involved.

Some guidelines on deciding when to use shell versus another language:

• If you are writing a script that is more than 200 lines long, or that uses non-straightforward control flow logic, consider rewriting it in a more structured language.
• When assessing the complexity of your code (e.g. to decide whether to switch languages) consider whether the code is easily maintainable by people other than its author.

Environment

File Extensions

Executables should have no extension (strongly preferred) or a .sh extension. Libraries must have a .sh extension and should not be executable.

STDOUT vs STDERR

All error messages should go to STDERR. This makes it easier to separate normal status from actual issues.

A function to print out error messages along with other status information is recommended.

err() {
    echo "[$(date +'%Y-%m-%dT%H:%M:%S%z')]: $*" >&2
}

if ! do_something; then
    err "Unable to do_something"
    exit 1
fi

Fail by default

All scripts must use set -eu (for POSIX sh) or set -euo pipefail (for Bash). Exceptions should be made on a case-by-case basis and feature inline comments explaining any omissions/disabling.

Comments

Every file must have a top-level comment including a brief overview of its contents.

Example:

#!/bin/bash
#
# Perform hot backups of Oracle databases.

Any function that is not both obvious and short must be commented. Any function in a library must be commented regardless of length or complexity. Comment tricky, non-obvious, interesting or important parts of your code.

Formatting

While you should follow the style that’s already there for files that you’re modifying, the following are required for any new code.

Indentation

Indent 4 spaces. No tabs. Use blank lines between blocks to improve readability.

Line Length and Long Strings

Maximum line length is 80 characters.

If you have to write strings that are longer than 80 characters, this should be done with a here document or an embedded newline if possible. Literal strings that have to be longer than 80 chars and can’t sensibly be split are OK, but it’s strongly preferred to find a way to make it shorter.

# DO use 'here document's
cat <<END
I am an exceptionally long
string.
END

# Embedded newlines are ok too
long_string="I am an exceptionally
long string."

Pipelines

Pipelines should be split one per line if they don’t all fit on one line. If a pipeline all fits on one line, it should be on one line.

If not, it should be split at one pipe segment per line with the pipe on the newline and a 4 space indent for the next section of the pipe. This applies to a chain of commands combined using | as well as to logical compounds using || and &&.

# All fits on one line
command1 | command2

# Long commands
command1 \
    | command2 \
    | command3 \
    | command4

Loops

Put ; do and ; then on the same line as the while, for or if. else should be on its own line and closing statements should be on their own line vertically aligned with the opening statement.

Example:

# If inside a function, consider declaring the loop variable as
# a local to avoid it leaking into the global environment:
# local dir
for dir in "${dirs_to_cleanup[@]}"; do
    if [[ -d "${dir}/${ORACLE_SID}" ]]; then
        log_date "Cleaning up old files in ${dir}/${ORACLE_SID}"
        rm "${dir}/${ORACLE_SID}/"*
        if (( $? != 0 )); then
            error_message
        fi
    else
        mkdir -p "${dir}/${ORACLE_SID}"
        if (( $? != 0 )); then
            error_message
        fi
    fi
done

Case statement

• A one-line alternative needs a space after the close parenthesis of the pattern and before the ;;.
• Long or multi-command alternatives should be split over multiple lines with the pattern, actions, and ;; on separate lines.

The matching expressions are indented one level from the case and esac. Multiline actions are indented another level. In general, there is no need to quote match expressions. Pattern expressions should not be preceded by an open parenthesis. Avoid the ;& and ;;& notations.

case "${expression}" in
    a)
        variable="…"
        some_command "${variable}" "${other_expr}" …
    ;;
    absolute)
        actions="relative"
        another_command "${actions}" "${other_expr}" …
    ;;
    *)
        error "Unexpected expression '${expression}'"
    ;;
esac

Simple commands may be put on the same line as the pattern and ;; as long as the expression remains readable. This is often appropriate for single-letter option processing. When the actions don’t fit on a single line, put the pattern on a line on its own, then the actions, then ;; also on a line of its own. When on the same line as the actions, use a space after the close parenthesis of the pattern and another before the ;;.

verbose='false'
aflag=''
bflag=''
files=''
while getopts 'abf:v' flag; do
    case "${flag}" in
        a) aflag='true' ;;
        b) bflag='true' ;;
        f) files="${OPTARG}" ;;
        v) verbose='true' ;;
        *) error "Unexpected option ${flag}" ;;
    esac
done

Quoting

• Always quote strings containing variables, command substitutions, spaces or shell meta characters, unless careful unquoted expansion is required or it’s a shell-internal integer.
• Curly braces are not required when quoting variables (i.e. "$var" is an acceptable quote). They can be useful for differentiating variables located close together, however.
• When quoting strings with variables, prefer quoting the entire string rather than the individual variable:

# Less readable:
echo "$bin_dir"/"$bin_file"

# More readable:
echo "$bin_dir/$bin_file"

• Use "$@" unless you have a specific reason to use $*, such as simply appending the arguments to a string in a message or log.

Features and Bugs

ShellCheck

The ShellCheck project identifies common bugs and warnings for your shell scripts. It is recommended for all scripts, large or small.

Command Substitution

Use $(command) instead of backticks.

Nested backticks require escaping the inner ones with \ . The $(command) format doesn’t change when nested and is easier to read.

Wildcard Expansion of Filenames

Use an explicit path when doing wildcard expansion of filenames.

As filenames can begin with a -, it’s a lot safer to expand wildcards with ./* instead of *. Additionally, use -- to signify the end of command options so that dashes are not inappropriately interpreted as commands.

# Here's the contents of the directory:
# -f  -r  somedir  somefile

# Incorrectly deletes almost everything in the directory by force
$ rm -v *

# As opposed to:
$ rm -v -- ./*
removed `./-f'
removed `./-r'
rm: cannot remove `./somedir': Is a directory
removed `./somefile'

Eval

eval should be avoided; Eval munges the input when used for assignment to variables and can set variables without making it possible to check what those variables were.

Arithmetic

Always use (( … )) or $(( … )) rather than let or $[ … ] or expr. Never use the $[ … ] syntax, the expr command, or the let built-in.

Naming Conventions

Function Names

Lower-case, with underscores to separate words. Parentheses are required after the function name. The keyword function is optional, but its use (or lack of) must be used consistently throughout a project.

# Single function
my_func() {
  …
}

Source Filenames

Lowercase, with underscores to separate words.

Variable Names

As for function names.

Variables names for loops should be similarly named for any variable you’re looping through.

for zone in "${zones[@]}"; do
  something_with "${zone}"
done

Constants and Environment Variable Names

All caps, separated with underscores, declared at the top of the file.

Constants and anything exported to the environment should be capitalized.

# Constant
readonly PATH_TO_FILES='/some/path'

# Both constant and environment
declare -xr ORACLE_SID='PROD'

Calling Commands

Checking Return Values

Always check return values and give informative return values.

For unpiped commands, use $? or check directly via an if statement to keep it simple.

Example:

if ! mv "${file_list[@]}" "$dest_dir/"; then
    err "Unable to move ${file_list[*]} to $dest_dir"
    exit 1
fi

# Or
mv "${file_list[@]}" "${dest_dir}/"
if (( $? != 0 )); then
    err "Unable to move ${file_list[*]} to $dest_dir"
    exit 1
fi

See also

• mw:Manual:Coding conventions
• Wikimedia Cloud Services team/Python coding
• Wikimedia Cloud Services team/ShellScript coding