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