mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 1 | These rules are fairly standard and boring. People will bitch about something |
| 2 | in here, no doubt. Get over it. Much of this was stolen from the Linux Kernel |
| 3 | coding style, because most of it makes good sense. If you disagree, that's OK, |
| 4 | but please stick to the rules anyway ;-) |
| 5 | |
| 6 | |
| 7 | Language |
| 8 | |
| 9 | Please use Python where possible. It's not the ideal language for everything, |
| 10 | but it's pretty good, and consistency goes a long way in making the project |
| 11 | maintainable. (Obviously using C or whatever for writing tests is fine). |
| 12 | |
| 13 | |
mbligh | 2ac475b | 2008-09-09 21:37:40 +0000 | [diff] [blame] | 14 | Base coding style |
| 15 | |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 16 | When writing python code, unless otherwise stated, stick to the python style |
mbligh | 2ac475b | 2008-09-09 21:37:40 +0000 | [diff] [blame] | 17 | guide (http://www.python.org/dev/peps/pep-0008/). |
| 18 | |
| 19 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 20 | Indentation & whitespace |
| 21 | |
| 22 | Format your code for an 80 character wide screen. |
| 23 | |
mbligh | c960fcf | 2008-06-18 19:58:57 +0000 | [diff] [blame] | 24 | Indentation is now 4 spaces, as opposed to hard tabs (which it used to be). |
| 25 | This is the Python standard. |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 26 | |
Tan Gao | 8aac17b | 2013-04-16 08:46:01 -0700 | [diff] [blame] | 27 | For hanging indentation, use 8 spaces plus all args should be on the new line. |
| 28 | |
| 29 | # Either of the following hanging indentation is considered acceptable. |
| 30 | YES: return 'class: %s, host: %s, args = %s' % ( |
| 31 | self.__class__.__name__, self.hostname, self.args) |
| 32 | |
| 33 | YES: return 'class: %s, host: %s, args = %s' % ( |
| 34 | self.__class__.__name__, |
| 35 | self.hostname, |
| 36 | self.args) |
| 37 | |
| 38 | # Do not use 4 spaces for hanging indentation |
| 39 | NO: return 'class: %s, host: %s, args = %s' % ( |
| 40 | self.__class__.__name__, self.hostname, self.args) |
| 41 | |
| 42 | # Do put all args on new line |
| 43 | NO: return 'class: %s, host: %s, args = %s' % (self.__class__.__name__, |
| 44 | self.hostname, self.args) |
| 45 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 46 | Don't leave trailing whitespace, or put whitespace on blank lines. |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 47 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 48 | Leave TWO blank lines between functions - this is Python, there are no clear |
| 49 | function end markers, and we need help. |
| 50 | |
| 51 | |
| 52 | Variable names and UpPeR cAsE |
| 53 | |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 54 | Use descriptive variable names where possible - it helps to make the code |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 55 | self documenting. |
| 56 | |
| 57 | Don't use CamelCaps style in most places - use underscores to separate parts |
| 58 | of your variable_names please. I shall make a bedgrudging exception for class |
| 59 | names I suppose, but I'll still whine about it a lot. |
| 60 | |
mbligh | d876f45 | 2008-12-03 15:09:17 +0000 | [diff] [blame] | 61 | |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 62 | Importing modules |
| 63 | |
| 64 | The order of imports should be as follows: |
| 65 | |
| 66 | Standard python modules |
| 67 | Non-standard python modules |
| 68 | Autotest modules |
| 69 | |
| 70 | Within one of these three sections, all module imports using the from |
| 71 | keyword should appear after regular imports. |
Christopher Wiley | d7445ef | 2014-12-05 13:26:05 -0800 | [diff] [blame] | 72 | Each module should be imported on its own line. |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 73 | Wildcard imports (from x import *) should be avoided if possible. |
| 74 | Classes should not be imported from modules, but modules may be imported |
| 75 | from packages, i.e.: |
| 76 | from common_lib import error |
| 77 | and not |
| 78 | from common_lib.error import AutoservError |
| 79 | |
| 80 | For example: |
Christopher Wiley | d7445ef | 2014-12-05 13:26:05 -0800 | [diff] [blame] | 81 | import os |
| 82 | import pickle |
| 83 | import random |
| 84 | import re |
| 85 | import select |
| 86 | import shutil |
| 87 | import signal |
| 88 | import subprocess |
| 89 | |
mbligh | 8bcd23a | 2009-02-03 19:14:06 +0000 | [diff] [blame] | 90 | import common # Magic autotest_lib module and sys.path setup code. |
| 91 | import MySQLdb # After common so that we check our site-packages first. |
Christopher Wiley | d7445ef | 2014-12-05 13:26:05 -0800 | [diff] [blame] | 92 | |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 93 | from common_lib import error |
| 94 | |
Christopher Wiley | d7445ef | 2014-12-05 13:26:05 -0800 | [diff] [blame] | 95 | |
mbligh | d876f45 | 2008-12-03 15:09:17 +0000 | [diff] [blame] | 96 | Testing None |
| 97 | |
| 98 | Use "is None" rather than "== None" and "is not None" rather than "!= None". |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 99 | This way you'll never run into a case where someone's __eq__ or __ne__ |
mbligh | d876f45 | 2008-12-03 15:09:17 +0000 | [diff] [blame] | 100 | method do the wrong thing |
| 101 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 102 | |
| 103 | Comments |
| 104 | |
| 105 | Generally, you want your comments to tell WHAT your code does, not HOW. |
| 106 | We can figure out how from the code itself (or if not, your code needs fixing). |
| 107 | |
| 108 | Try to describle the intent of a function and what it does in a triple-quoted |
| 109 | (multiline) string just after the def line. We've tried to do that in most |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 110 | places, though undoubtedly we're not perfect. A high level overview is |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 111 | incredibly helpful in understanding code. |
| 112 | |
| 113 | |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 114 | Hardcoded String Formatting |
| 115 | |
| 116 | Strings should use only single quotes for hardcoded strings in the code. Double |
| 117 | quotes are acceptable when single quote is used as part of the string. |
| 118 | Multiline string should not use '\' but wrap the string using parenthesises. |
| 119 | |
| 120 | REALLY_LONG_STRING = ('This is supposed to be a really long string that is ' |
| 121 | 'over 80 characters and does not use a slash to ' |
| 122 | 'continue.') |
| 123 | |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 124 | Docstrings |
| 125 | |
| 126 | Docstrings are important to keep our code self documenting. While it's not |
| 127 | necessary to overdo documentation, we ask you to be sensible and document any |
| 128 | nontrivial function. When creating docstrings, please add a newline at the |
showard | 25f056a | 2009-11-23 20:22:50 +0000 | [diff] [blame] | 129 | beginning of your triple quoted string and another newline at the end of it. If |
| 130 | the docstring has multiple lines, please include a short summary line followed |
| 131 | by a blank line before continuing with the rest of the description. Please |
| 132 | capitalize and punctuate accordingly the sentences. If the description has |
| 133 | multiple lines, put two levels of indentation before proceeding with text. An |
| 134 | example docstring: |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 135 | |
| 136 | def foo(param1, param2): |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 137 | """ |
showard | 25f056a | 2009-11-23 20:22:50 +0000 | [diff] [blame] | 138 | Summary line. |
| 139 | |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 140 | Long description of method foo. |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 141 | |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 142 | @param param1: A thing called param1 that is used for a bunch of stuff |
| 143 | that has methods bar() and baz() which raise SpamError if |
| 144 | something goes awry. |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 145 | |
| 146 | @returns a list of integers describing changes in a source tree |
| 147 | |
| 148 | @raises exception that could be raised if a certain condition occurs. |
| 149 | |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 150 | """ |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 151 | |
| 152 | The tags that you can put inside your docstring are tags recognized by systems |
| 153 | like doxygen. Not all places need all tags defined, so choose them wisely while |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 154 | writing code. Generally (if applicable) always list parameters, return value |
| 155 | (if there is one), and exceptions that can be raised to each docstring. |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 156 | |
| 157 | @author - Code author |
| 158 | @param - Parameter description |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 159 | @raise - If the function can throw an exception, this tag documents the |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 160 | possible exception types. |
Simran Basi | c7330bd | 2013-05-31 09:23:50 -0700 | [diff] [blame] | 161 | @raises - same as @raise. |
| 162 | @return - Return value description |
| 163 | @returns - Same as @return |
| 164 | @see - Reference to what you have done |
| 165 | @warning - Call attention to potential problems with the code |
| 166 | @var - Documentation for a variable or enum value (either global or as a |
| 167 | member of a class) |
| 168 | @version - Version string |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 169 | |
mbligh | 3bdba92 | 2010-05-03 18:02:54 +0000 | [diff] [blame] | 170 | When in doubt refer to: http://doxygen.nl/commands.html |
| 171 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 172 | Simple code |
| 173 | |
| 174 | Keep it simple; this is not the right place to show how smart you are. We |
| 175 | have plenty of system failures to deal with without having to spend ages |
| 176 | figuring out your code, thanks ;-) Readbility, readability, readability. |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 177 | I really don't care if other things are more compact. |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 178 | |
| 179 | "Debugging is twice as hard as writing the code in the first place. Therefore, |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 180 | if you write the code as cleverly as possible, you are, by definition, not |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 181 | smart enough to debug it." Brian Kernighan |
| 182 | |
| 183 | |
| 184 | Function length |
| 185 | |
| 186 | Please keep functions short, under 30 lines or so if possible. Even though |
| 187 | you are amazingly clever, and can cope with it, the rest of us are all stupid, |
| 188 | so be nice and help us out. To quote the Linux Kernel coding style: |
| 189 | |
| 190 | Functions should be short and sweet, and do just one thing. They should |
| 191 | fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, |
| 192 | as we all know), and do one thing and do that well. |
| 193 | |
| 194 | |
mbligh | 900b6c1 | 2008-01-14 16:56:47 +0000 | [diff] [blame] | 195 | Exceptions |
| 196 | |
| 197 | When raising exceptions, the preferred syntax for it is: |
| 198 | |
| 199 | raise FooException('Exception Message') |
| 200 | |
| 201 | Please don't raise string exceptions, as they're deprecated and will be removed |
| 202 | from future versions of python. If you're in doubt about what type of exception |
| 203 | you will raise, please look at http://docs.python.org/lib/module-exceptions.html |
| 204 | and client/common_lib/error.py, the former is a list of python built in |
| 205 | exceptions and the later is a list of autotest/autoserv internal exceptions. Of |
| 206 | course, if you really need to, you can extend the exception definitions on |
| 207 | client/common_lib/error.py. |
| 208 | |
| 209 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 210 | Submitting patches |
| 211 | |
| 212 | Generate universal diffs. Email them to autotest@test.kernel.org. |
| 213 | Most mailers now break lines and/or changes tabs to spaces. If you know how |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 214 | to avoid that - great, put your patches inline. If you're not sure, just |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 215 | attatch them, I don't care much. Please base them off the current version. |
| 216 | |
| 217 | Don't worry about submitting patches to a public list - everybody makes |
| 218 | mistakes, especially me ... so get over it and don't worry about it. |
| 219 | (though do give your changes a test first ;-)) |