http://goto/factory-style-guide
There are lots of Google, Chromium, and Chromium OS style guides out there. The ones most relevant for factory work are:
Help! Which one do I use?
First of all, don‘t sweat it too much. You do need to be familiar with the style guides for the languages you use, but there’s a lot of information in there and you're not going to remember everything the first time. Try your best and use common sense!
Follow the Chromium OS Python style guide, which says that Chromium OS code should be “PEP-8 with exceptions”. The biggest exceptions are that,
MixedCase
instead of lower_case
for function names.PEP-8 says that acronyms are written in upper-case: HTTPServerError
not HttpServerError
. We follow PEP-8 here.
You should also read and abide by the Google Python style guide, especially the sections about comments. (The “official” Chromium OS policy says that the Google Python style guide isn't an authority, but in general it has lots of good points and you should follow it unless there is a strong reason not to do so.) There are a few places where you need to ignore the Google Python style guide:
MixedCase
instead of lower_case
for function names.HTTPServerError
not HttpServerError
.#!/usr/bin/env python3
or #!/usr/bin/python3
for shebang.Finally, you must use pylint. In platform/factory, you can run make lint
to do this. platform/factory/Makefile has a blocklist for files that are not yet pylint-compliant; if you make substantial changes to an existing file, please fix lint problems and remove the file from the blocklist.
The recommended way is:
#!/usr/bin/env python3
Some of the factory code, for example setup/*, need to run on a non-ChromiumOS machine in factory. It may be any Linux distribution, or even Windows Box. So we allow using env in shebang to reduce compatibility issues. However on Linux, parameters in shebang when using env will be considered as file name for execution, so we need to allow both (using env, or just python). See https://chromium-review.googlesource.com/265172 for some discussion.
Never use tempfile.mkstemp
because it leaves an opened fd and would cause leaks easily. Try the utility functions in cros.factory.utils.files_utils
: CreateTemporaryFile
and UnopenedTemporaryFile
first. (They are implemented by tempfile.NamedTemporaryFile(delete=False)
)
If you want to catch “most exceptions”, do except Exception:
instead of except:
, because the later one also catches SystemExit
and KeyboardInterrupt
.
make lint
should warn you about this.
The existing guidelines didn‘t set a very precise rule about how to name unused variables. There were some discussion and here’s our conclusion.
For variables inside a function, follow unused_*
naming style.
For variables as function parameter (usually due to overriding methods in a class), name as usual then del var
, and del should be in the beginning of that function.
Example:
class B(A): def DoSomething(self, myarg): del myarg # unused ... fd, unused_fname = temp.mkstemp() writelog(fd) ...
For easy cutting-and-pasting, here's the header you should use for Python files (based on this).
#!/usr/bin/env python3 # Copyright 2020 The Chromium OS Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file.
NOTE: factory_common
has been removed (b/112251287, CL:1837164).
If there are no special reasons, a symbolic link should be created under bin/
folder. The symbolic link should be linked to ../py/cli/factory_env.py
. And a file or symbolic link should be created under py/cli/
, which is the real implementation of the command line tool. For example,
platform/factory |-- bin/ | `-- image_tool -> ../py/cli/factory_env.py `-- py/ |-- cli/ | |-- factory_env.py | `-- image_tool.py -> ../tools/image_tool.py `-- tools/ `-- image_tool.py
The factory_env.py
script detects the real python script to execute (py/tools/image_tool.py
in this case), and inject the factory package folder into system path before running the real python script.
Imports should be split into following sections:
cros.factory
) packagescros.factory.external
packagesIn each section, import lines should be sorted, the sorting key for each import line is constructued as following:
import a.b.c # sorting key: 'a.b.c' from x.y import z # sorting key: 'x.y.z' import a.b.c as xyz # sorting key: 'a.b.c', "as xyz" is ignored. from x.y import z as abc # sorting key: 'x.y.z', "as abc" is ignored.
sort_import.vim is a Vim plugin implementation to sort imports alphabetically.
argparse
.argparse
for anything with a Main()
method. Even if you have no command-line arguments, at least this will display a help screen. And trust me, you will add command-line arguments some day anyway :)--shopfloor-url
rather than --shopfloor_url
). This is consistent with the vast majority of UNIX programs out there.--no-*
and action='store_false'
, e.g.parser.add_argument('--no-upload', action='store_false', dest='upload')
You then test it with:
if args.upload: ... do some stuff ...
This is a lot more intuitive than testing negative conditions (if not args.no_upload
).
Follow the Chromium OS Web Development style guide.
Unfortunately, a lot of code was written either before we had real coding standards, or written according to autotest coding standards and then moved away. Such is life. Please use the official conventions when writing new files (or possibly when editing in an existing file), but you can continue to use the old conventions in existing files. Use your judgment!
Please see Inclusive Chromium Code.