-
Notifications
You must be signed in to change notification settings - Fork 18
coding standards
This (currently empty) section includes standards we've consensed on.
These are proposals that the team has not yet consensed on:
Follow the tahoe-lafs Coding Standards whenever possible.
Exception: Sometimes those standards assume the allmydata
codebase which is not always feasible for LeastAuthority repositories.
In that case, we approximate in an ad-hoc manner until we do that too often. Then we either write a redundant library or split out functionality from allmydata
. An example is allmydata.util.assertutil
.
Use twisted.python.filepath
instead of os.path
if possible:
- If Twisted is not a dependency, strongly consider making it a dependency even just to use
twisted.python.filepath
. - Create
FilePath
objects as close to the string input as possible.
If Twisted cannot be a dependency:
-
Always translate paths to absolute paths as close to the string input as possible with
os.path.abspath
.- Rationale: This makes logs and error messages unambiguous.
-
Always derive child paths from parent paths when possible using
os.path.join
.-
Rationale:
os.path.join(a, b)
works regardless of ifa
ends with a path separator or not. -
Rationale: Traversing up to parents is often surprising to users (ie: If I run
foo ./my/path
and this reads or changesfoo ./blah
I am often surprised except for well known exceptions like home directory dot paths).
-
Rationale:
-
Eschew string manipulation of paths:
-
Never use python string manipulation to compute or change details of directory hierarchy or extension, always rely on
os.path.join
andos.path.splitext
to maintain hierarchy and extensions. -
When synthesizing or parsing path components with string operations, always operate on a single "component" rather than a path which spans hierarchy.
-
eg:
- instead of:
logpath = '%s/%s-%s.log' % (logdir, componentname, timestamp)
- use:
logpath = os.path.join(logdir, '%s-%s.log' % (componentname, timestamp))
- In both of these examples, how do you know neither
componentname
nortimestamp
contain a path separator? (-It requires more context than these snippets and is a burden for code readers.)
- instead of:
-
-
Rationale: String manipulation for hierarchy manipulation is redundant with
os.path
and very likely to have bugs whichos.path
does not. -
Rationale: The intent is clearer when reading code. eg:
os.path.join(a, b)
vs'/'.join([a, b])
etc...
-
Eschew shell=True
in calls to subprocess
APIs. This is a security hazard and rarely provides any benefit. In the case that shell behavior is relied upon, consider rewriting those features explicitly in python.
Warning
Note that shell=True
is implied if the args
parameter is a single string.
Warning
Even when args
is a sequences of strings, shell=True
is still possible. (I like to always pass shell=False
explicitly, even though that is the default if args
is a sequence of strings. This is the kind of "flexible" python API style I abhor.)
If there are any other cases where shell escaping is used, this is almost certainly a cause for concern for security and correctness. Looking for the pipes
module which has shell_quote
.
If we have a variable which refers to part of a git repository it shall have the following form:
$(REPO_NAME)_repo_gitdir
OR
$(REPO_NAME)_repo_workdir
depending on which component it refers to.
We'll always derive the
from twisted.python.filepath import FilePath
$(REPO_NAME)_repo_workdir = FilePath( $(DIRECTORY_NAME_STRING) )
$(REPO_NAME)_repo_gitdir = $(REPO_NAME)_repo_workdir.child('.git')