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. |
| 47 | Leave TWO blank lines between functions - this is Python, there are no clear |
| 48 | function end markers, and we need help. |
| 49 | |
| 50 | |
| 51 | Variable names and UpPeR cAsE |
| 52 | |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 53 | Use descriptive variable names where possible - it helps to make the code |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 54 | self documenting. |
| 55 | |
| 56 | Don't use CamelCaps style in most places - use underscores to separate parts |
| 57 | of your variable_names please. I shall make a bedgrudging exception for class |
| 58 | names I suppose, but I'll still whine about it a lot. |
| 59 | |
jamesren | d0e1716 | 2010-07-29 17:37:32 +0000 | [diff] [blame] | 60 | Importing modules |
| 61 | |
| 62 | The order of imports should be as follows: |
| 63 | |
| 64 | Standard python modules |
| 65 | Non-standard python modules |
| 66 | Autotest modules |
| 67 | |
| 68 | Within one of these three sections, all module imports using the from |
| 69 | keyword should appear after regular imports. |
| 70 | Modules should be lumped together on the same line. |
| 71 | Wildcard imports (from x import *) should be avoided if possible. |
| 72 | Classes should not be imported from modules, but modules may be imported |
| 73 | from packages, i.e.: |
| 74 | from common_lib import error |
| 75 | and not |
| 76 | from common_lib.error import AutoservError |
| 77 | |
| 78 | For example: |
| 79 | import os, pickle, random, re, select, shutil, signal, StringIO, subprocess |
| 80 | import sys, time, urllib, urlparse |
| 81 | import MySQLdb |
| 82 | from common_lib import error |
| 83 | |
mbligh | d876f45 | 2008-12-03 15:09:17 +0000 | [diff] [blame] | 84 | |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 85 | Importing modules |
| 86 | |
| 87 | The order of imports should be as follows: |
| 88 | |
| 89 | Standard python modules |
| 90 | Non-standard python modules |
| 91 | Autotest modules |
| 92 | |
| 93 | Within one of these three sections, all module imports using the from |
| 94 | keyword should appear after regular imports. |
| 95 | Modules should be lumped together on the same line. |
| 96 | Wildcard imports (from x import *) should be avoided if possible. |
| 97 | Classes should not be imported from modules, but modules may be imported |
| 98 | from packages, i.e.: |
| 99 | from common_lib import error |
| 100 | and not |
| 101 | from common_lib.error import AutoservError |
| 102 | |
| 103 | For example: |
| 104 | import os, pickle, random, re, select, shutil, signal, StringIO, subprocess |
| 105 | import sys, time, urllib, urlparse |
mbligh | 8bcd23a | 2009-02-03 19:14:06 +0000 | [diff] [blame] | 106 | import common # Magic autotest_lib module and sys.path setup code. |
| 107 | import MySQLdb # After common so that we check our site-packages first. |
mbligh | 7654c82 | 2008-04-04 15:12:48 +0000 | [diff] [blame] | 108 | from common_lib import error |
| 109 | |
mbligh | d876f45 | 2008-12-03 15:09:17 +0000 | [diff] [blame] | 110 | Testing None |
| 111 | |
| 112 | Use "is None" rather than "== None" and "is not None" rather than "!= None". |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 113 | 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] | 114 | method do the wrong thing |
| 115 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 116 | |
| 117 | Comments |
| 118 | |
| 119 | Generally, you want your comments to tell WHAT your code does, not HOW. |
| 120 | We can figure out how from the code itself (or if not, your code needs fixing). |
| 121 | |
| 122 | Try to describle the intent of a function and what it does in a triple-quoted |
| 123 | (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] | 124 | places, though undoubtedly we're not perfect. A high level overview is |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 125 | incredibly helpful in understanding code. |
| 126 | |
| 127 | |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 128 | Docstrings |
| 129 | |
| 130 | Docstrings are important to keep our code self documenting. While it's not |
| 131 | necessary to overdo documentation, we ask you to be sensible and document any |
| 132 | nontrivial function. When creating docstrings, please add a newline at the |
showard | 25f056a | 2009-11-23 20:22:50 +0000 | [diff] [blame] | 133 | beginning of your triple quoted string and another newline at the end of it. If |
| 134 | the docstring has multiple lines, please include a short summary line followed |
| 135 | by a blank line before continuing with the rest of the description. Please |
| 136 | capitalize and punctuate accordingly the sentences. If the description has |
| 137 | multiple lines, put two levels of indentation before proceeding with text. An |
| 138 | example docstring: |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 139 | |
| 140 | def foo(param1, param2): |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 141 | """ |
showard | 25f056a | 2009-11-23 20:22:50 +0000 | [diff] [blame] | 142 | Summary line. |
| 143 | |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 144 | Long description of method foo. |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 145 | |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 146 | @param param1: A thing called param1 that is used for a bunch of stuff |
| 147 | that has methods bar() and baz() which raise SpamError if |
| 148 | something goes awry. |
mbligh | 3bdba92 | 2010-05-03 18:02:54 +0000 | [diff] [blame] | 149 | @return a list of integers describing changes in a source tree |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 150 | ... |
| 151 | """ |
mbligh | 5cad50f | 2009-06-08 16:50:51 +0000 | [diff] [blame] | 152 | |
| 153 | The tags that you can put inside your docstring are tags recognized by systems |
| 154 | like doxygen. Not all places need all tags defined, so choose them wisely while |
| 155 | writing code. |
| 156 | |
| 157 | @author - Code author |
| 158 | @param - Parameter description |
| 159 | @return - Return value description |
| 160 | @see - Reference to what you have done |
| 161 | @todo - Things that still need to be worked out |
| 162 | @version - Version string |
| 163 | @warning - Call attention to potential problems with the code |
| 164 | @raises - If the function can throw an exception, this tag documents the |
| 165 | possible exception types. |
| 166 | |
mbligh | 3bdba92 | 2010-05-03 18:02:54 +0000 | [diff] [blame] | 167 | When in doubt refer to: http://doxygen.nl/commands.html |
| 168 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 169 | Simple code |
| 170 | |
| 171 | Keep it simple; this is not the right place to show how smart you are. We |
| 172 | have plenty of system failures to deal with without having to spend ages |
| 173 | figuring out your code, thanks ;-) Readbility, readability, readability. |
mbligh | 5220736 | 2009-09-03 20:54:29 +0000 | [diff] [blame] | 174 | I really don't care if other things are more compact. |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 175 | |
| 176 | "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] | 177 | 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] | 178 | smart enough to debug it." Brian Kernighan |
| 179 | |
| 180 | |
| 181 | Function length |
| 182 | |
| 183 | Please keep functions short, under 30 lines or so if possible. Even though |
| 184 | you are amazingly clever, and can cope with it, the rest of us are all stupid, |
| 185 | so be nice and help us out. To quote the Linux Kernel coding style: |
| 186 | |
| 187 | Functions should be short and sweet, and do just one thing. They should |
| 188 | fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, |
| 189 | as we all know), and do one thing and do that well. |
| 190 | |
| 191 | |
mbligh | 900b6c1 | 2008-01-14 16:56:47 +0000 | [diff] [blame] | 192 | Exceptions |
| 193 | |
| 194 | When raising exceptions, the preferred syntax for it is: |
| 195 | |
| 196 | raise FooException('Exception Message') |
| 197 | |
| 198 | Please don't raise string exceptions, as they're deprecated and will be removed |
| 199 | from future versions of python. If you're in doubt about what type of exception |
| 200 | you will raise, please look at http://docs.python.org/lib/module-exceptions.html |
| 201 | and client/common_lib/error.py, the former is a list of python built in |
| 202 | exceptions and the later is a list of autotest/autoserv internal exceptions. Of |
| 203 | course, if you really need to, you can extend the exception definitions on |
| 204 | client/common_lib/error.py. |
| 205 | |
| 206 | |
mbligh | 71d338a | 2007-10-08 05:05:50 +0000 | [diff] [blame] | 207 | Submitting patches |
| 208 | |
| 209 | Generate universal diffs. Email them to autotest@test.kernel.org. |
| 210 | 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] | 211 | 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] | 212 | attatch them, I don't care much. Please base them off the current version. |
| 213 | |
| 214 | Don't worry about submitting patches to a public list - everybody makes |
| 215 | mistakes, especially me ... so get over it and don't worry about it. |
| 216 | (though do give your changes a test first ;-)) |