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