blob: 06612190a55ecb33d74485330e5fe8a21c88cebf [file] [log] [blame]
Ben Chanfad4a0b2012-04-18 15:49:59 -07001Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
mukesh agrawal102a7752011-07-28 14:57:14 -07002Use of this source code is governed by a BSD-style license that can be
3found in the LICENSE file.
4
5To keep the shill source code consistent, please follow the conventions below:
6
7- Use the Chromium Coding Style, as described at
8 http://www.chromium.org/developers/coding-style.
9
10 If you use Emacs, the Google C Style mode will help you with the formatting
11 aspects of style. (Chromium Style generally follows Google Style). Get the
12 Emacs mode at
13 http://google-styleguide.googlecode.com/svn/trunk/google-c-style.el
14
15 Note that we've deviated from the Chromium style in the following
16 ways. In these cases, follow the shill style, for consistency with
17 the rest of the shill code:
18
19 - We denote pointer and reference variables by placing the '*' and '&'
20 adjacent to the variable name, rather than the type. E.g.
21
22 void *bar
23
24 rather than
25
26 void* bar
27
28 - <no other deviations documented yet>
29
30- When working with DBus::Variant:
31 - Read data via the appropriate named method, rather than depending on
32 implicit conversion. E.g.,
33
mukesh agrawal6e277772011-09-29 15:04:23 -070034 ::DBus::Variant var;
mukesh agrawal102a7752011-07-28 14:57:14 -070035 int8 data = var.reader().get_byte();
36
37 rather than
38
mukesh agrawal6e277772011-09-29 15:04:23 -070039 ::DBus::Variant var;
mukesh agrawal102a7752011-07-28 14:57:14 -070040 int8 data = var;
41
42 RATIONALE: The explicit version is only marginally longer than the
43 implicit version, and does not require the reader to understand C++
44 conversion rules.
45
46 - Where there is no named method, call the appropriate cast operator
47 explicitly. E.g.
48
mukesh agrawal6e277772011-09-29 15:04:23 -070049 ::DBus::Variant var;
mukesh agrawal102a7752011-07-28 14:57:14 -070050 vector<unsigned int> data = var.operator vector<unsigned int>();
51
52 RATIONALE: Calling the cast operator explicitly avoids conflicts with
53 constructors that might also be used to make the conversion. It also
54 avoids requiring that the reader understand C++ conversion rules.
55
mukesh agrawal6e277772011-09-29 15:04:23 -070056 - Write data via the appropriate named method. E.g.,
57
58 ::DBus::Variant var;
59 int16_t data;
60 var.writer().append_int16(data);
61
62 rather than
63
64 ::DBus::Variant var;
65 int16_t data;
66 var.writer() << data;
67
68 RATIONALE: Similarly as for reading, the explicit version is only
69 marginally longer, and does not require the reader to understand
70 overload resolution.
71
72 - Where there is no named method, write by using the stream
73 insertion operator. E.g.
74
75 ::DBus::Variant var;
76 ::DBus::MessageIter writer;
77 map<string, string> data;
78 writer = var.writer();
79 writer << data;
80
81 RATIONALE: This case is somewhat unfortunate, because it's not as
82 clear as its analogue for reading. However, the alternative is to
83 duplicate the code of the stream insertion operator overloads.
84
85 Note that the writer can't be omitted. E.g.
86
87 ::DBus::Variant var;
88 map<string, string> data;
89 var.writer() << data;
90
mukesh agrawal15908392011-11-16 18:29:25 +000091 does not work. For an explanation of why the local variable
92 |writer| is needed, see the comment in
93 DBusAdaptor::ByteArraysToVariant.
mukesh agrawal6e277772011-09-29 15:04:23 -070094
mukesh agrawal102a7752011-07-28 14:57:14 -070095- When deferring work from a signal handler (e.g. a D-Bus callback) to
96 the event loop, name the deferred work function by adding "Task" to
97 the name of the function deferring the work. E.g.
98
99 void Modem::Init() {
100 dispatcher_->PostTask(task_factory_.NewRunnableMethod(&Modem::InitTask));
101 }
102
103 RATIONALE: The naming convention makes the relationship between the signal
104 handler and the task function obvious, at-a-glance.
Ben Chanfad4a0b2012-04-18 15:49:59 -0700105
Ben Chan80326f32012-05-04 17:51:32 -0700106- C++ exceptions are not allowed in the code. An exception to this rule is
107 that try-catch blocks may be used in various D-Bus proxy classes to handle
108 DBus::Error exceptions thrown by the D-Bus C++ code. C++ exceptions should
109 be caught by const reference in general.
110
Ben Chanfad4a0b2012-04-18 15:49:59 -0700111- When adding verbose log messages for debug purposes, use the SLOG marco and
Christopher Wileyb691efd2012-08-09 13:51:51 -0700112 its variants (see logging.h for details).
Ben Chanfad4a0b2012-04-18 15:49:59 -0700113
114 - Choose the appropriate scope and verbose level for log messages. E.g.
115
116 SLOG(WiFi, 1) << message; // for WiFi related code
117
118 - Before defining a new scope, check if any existing scope defined in
119 scope_logger.h already fulfills the needs.
120
121 - To add a new scope:
122 1. Add a new value to the Scope enumerated type in scope_logger.h.
123 Keep the values sorted as instructed in the header file.
124 2. Add the corresponding scope name to the kScopeNames array in
125 scope_logger.cc.
126 3. Update the GetAllScopeNames test in scope_logger_unittest.cc.
mukesh agrawalbebf1b82013-04-23 15:06:33 -0700127
128- When adding externally visible (i.e. via RPC) properties to an object,
129 make sure that a) its setter emits any change notification required by
130 Chrome, and that b) its setter properly handles no-op changes.
131
132 Test that the property changes are handled correctly by adding test
133 cases similar to those in CellularServiceTest.PropertyChanges, and
Paul Stewart6db7b242014-05-02 15:34:21 -0700134 CellularServiceTest.CustomSetterNoopChange.
135
136- When performing trivial iteration through a container, prefer using
137 range based for loops, preferably:
138
139 for (const auto &element : container) {
140
141 Remove "const" where necessary if the element will be modified during
142 the loop. Removal of the "const" and reference for trivial types is
143 allowed but not necessary.