It seems very common in other Go based projects to place the local packages in the import statement last. Currently Ceph-CSI expects the imports to group the local packages immediately after standard packages. This exception compared to other projects often requires new contributors to 'correct' their PR. Following a more common convention for grouping imports should make it a little easier to contribute to the project. Signed-off-by: Niels de Vos <ndevos@ibm.com>
3.7 KiB
Coding Conventions
Please follow coding conventions and guidelines described in the following documents:
- Go proverbs - highly recommended read
- CodeReviewComments
- Effective Go
- How to Write a Git Commit Message
Here's a list of some more specific conventions that are often followed in the code and will be pointed out in the review process:
General
- Keep variable names short for variables that are local to the function.
- Do not export a function or variable name outside the package until you have an external consumer for it.
Imports
We prefer the following convention for specifying imports:
<import standard library packages>
<import third-party packages>
<import ceph-csi packages>
Example:
import (
"os"
"path"
"strings"
"time"
"github.com/pborman/uuid"
"github.com/ceph/ceph-csi/internal/util"
)
Error Handling
- Use variable name
err
to denote error variable during a function call. - Reuse the previously declared
err
variable as long as it is in scope. For example, do not useerrWrite
orerrRead
. - Do not panic() for errors that can be bubbled up back to user. Use panic() only for fatal errors which shouldn't occur.
- Do not ignore errors using
_
variable unless you know what you're doing. - Error strings should not start with a capital letter.
- If error requires passing of extra information, you can define a new type
- Error types should end with
Error
.
Logging
- The inner-most utility functions should never log. Logging must almost always
be done by the caller on receiving an
error
. - Always use log level
DEBUG
to provide useful diagnostic information to developers or sysadmins. - Use log level
INFO
to provide information to users or sysadmins. This is the kind of information you'd like to log in an out-of-the-box configuration in happy scenario. - Use log level
WARN
when something fails but there's a workaround or fallback or retry for it and/or is fully recoverable. - Use log level
ERROR
when something occurs which is fatal to the operation, but not to the service or application.
Wrap long lines
At present, we restrict the number of chars in a line to 120
which is the
default value for the lll
linter check we have in CI. If your source code line
or comment goes beyond this limit please try to organize it in such a way it
is within the line length limit and help on code reading.
Example:
_, err := framework.RunKubectl(cephCSINamespace, "delete", "cm", "ceph-csi-encryption-kms-config", "--namespace", cephCSINamespace, "--ignore-not-found=true")
Instead of above long function signature, we can organize it to something like below which is clear and help on easy code reading.
_, err := framework.RunKubectl(
cephCSINamespace,
"delete",
"cm",
"ceph-csi-encryption-kms-config",
"--namespace",
cephCSINamespace,
"--ignore-not-found=true")
Mark Down Rules
-
MD014 - Dollar signs used before commands without showing output
The dollar signs are unnecessary, it is easier to copy and paste and less noisy if the dollar signs are omitted. Especially when the command doesn't list the output, but if the command follows output we can use '$ ' (dollar+space) mainly to differentiate between command and its output.
scenario 1: when command doesn't follow output
cd ~/work
scenario 2: when command follow output (use dollar+space)
$ ls ~/work file1 file2 dir1 dir2 ...