-
Notifications
You must be signed in to change notification settings - Fork 0
/
general-guidelines.tex
196 lines (153 loc) · 11.4 KB
/
general-guidelines.tex
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
This document details some rules for future developed code.
Keep in mind that the code will be read by humans and will be eventually modified by them (you may not be the last person to use it).
It must be readable, understandable and reusable.\\
The most important rule about coding is the \textbf{consistency}.
Hence, if a project already has its own format, do not impose yours, and try to adapt yourself.
There are several reasons for that:
\begin{itemize}
\item Correcting everything will take you a long time that you may not have
\item Temporarily mixing two coding styles is not a good thing to do.
\end{itemize}
Hence, if you are a visitor/occasional developer of a package, you may signal to the main developers that some rules have been broken. But don't do it by yourself,
Prefer asserting the consistency inside one package rather than the consistency inside the whole project.
However, when creating a new package, please follow those coding guidelines.
\chapter{General guidelines}
\label{section:general-guidelines}
\begin{itemize}[noitemsep,topsep=0pt,parsep=0pt,partopsep=0pt]
\item Ensure the consistency of each project (this is \textbf{\textit{that}} important).
\item Maintain the documentation up-to-date (use doxygen, sphinx...), and make your code understandable.
\item Make the validation of the code easier by adding unit-tests.
\item Use versioning tools (git or svn, depending on the type of data), tag versions that work and release often.
\item Keep the code clean. Remove useless and dead code (especially if it is versioned).
\item Do not neglect debugging tools (e.g. make the debug easier by using assertion (C++)).
\end{itemize}
\section{General shape of package}
All packages must contain the same information (author list, description, copyright\ldots), but depending on the type of package
(generic package or ROS package), those informations will be detailed in different locations.
\subsection{Generic package}
Packages independent from ROS should at least contain the following elements:
\paragraph{README(.md)}
The README(.md) file is the first file a newcomer will consult.
The .md extension should be added if you use markdown to make the reading of the file easier.
It should define all the informations relative to the project:
\begin{itemize}[noitemsep,topsep=0pt,parsep=0pt,partopsep=0pt]
\item The purpose of the package.
\item The use/compilation informations: on which OS does it work? What are the dependencies of the project (with the version if it matters).
\item The related papers.
\item The origin of the sources, and the project management web application (github, redmine \ldots)
\end{itemize}
It is important to detail on which OS the code has been tested, especially for C++.
While using cmake and avoiding OS specific code or header inclusion are good practices,
they do not automatically ensure the compatibility between the platforms.
For example, on windows systems, there are some additional routines to export the symbols of the libraries.
On Mac OS, the linkage verification is stricter than on ubuntu (where the linkage can be solved at the compilation).
Also, the double precision handling may be different between 64 bits and 32 bits systems, which can also modify the behavior of a program.
The purpose is mainly to say to users that there are no maintainers for some OS, and that they will be on their own if they want to port the code.
\paragraph{Authors}
The list of contributing authors can be added at the top of each files.
The order to keep is the alphabetical order of last name, then first name.
Since this can be a daunting task, an alternative is to define an AUTHOR file in the root directory of your package, in which you can indicate the corresponding(s) author(s) and list all the contributors and a summed up detail of their contribution.\\
To simply list the contributors, you can use the following script:
\begin{verbatim}
#!/bin/sh
# Source: http://goo.gl/Y4UWR
set -e
# Get a list of authors ordered by number of commits
# and remove the commit count column
AUTHORS=$(git --no-pager shortlog -nse | cut -f 2-)
if [ -z "$AUTHORS" ] ; then
echo "Authors list was empty"
exit 1
fi
# Display the authors list and write it to the file
echo "$AUTHORS" | tee "$(git rev-parse --show-toplevel)/AUTHORS"
\end{verbatim}
\paragraph{Copyright}
The full text statements of all licenses used in the project should be placed in the LICENSES directory at the top of the repository.
If you add a package under a license that is not included in LICENSES, add the license statement.
The code developed in the frame of the robohow project should be open source.
The licenses used in this purpose are the following ones (from the more restrictive to the least one).
\begin{itemize}[noitemsep,topsep=0pt,parsep=0pt,partopsep=0pt]
\item GPL\footnote{\url{http://www.gnu.org/licenses/gpl.html}}, permits the use of the library only for free programs.
\item LGPL\footnote{\url{http://www.gnu.org/copyleft/lesser.html}} permits the use of the library in proprietary programs.
\item BSD \footnote{\url{http://opensource.org/licenses/BSD-3-Clause}}
\end{itemize}
\subsection{ROS package}
The organization of a ROS package is slightly different from a generic package, since
more informations are gathered in the ROS specific files manifest.xml and stack.xml.
Hence ROS packages should at least contain the following elements:
\paragraph{manifest.xml or stack.xml}
The manifest.xml (for a package) / stack.xml (for a stack) files should detail the following informations:
\begin{itemize}[noitemsep,topsep=0pt,parsep=0pt,partopsep=0pt]
\item Author list
\item Description
\item License
\item Url of the project.
\end{itemize}
As a result, please \textbf{DO NOT} create files such as AUTHOR, in order to avoid inconsistencies with the manifest.xml file.
\paragraph{README(.md)}
Since the description of the package is now in the manifest/stack files, the README file now contains less informations:
\begin{itemize}[noitemsep,topsep=0pt,parsep=0pt,partopsep=0pt]
\item The use/compilation informations: on which OS does it work? What are the dependencies of the project (with the version if it matters).
\item The related papers.
\end{itemize}
% According to ROS guidelines\url{http://www.ros.org/wiki/DevelopersGuide}
\section{File convention}
\subsection{Line endings}
All text files must be normalized so that lines terminate in the unix style (LF).
Mixture of termination styles will sooner or later create an issue in the versioning tool when the end-of-line character will change, and make the history unusable.
Hence, avoid committing files that terminate in CRLF (windows ending) or CR (mac ending) since such a modification is likely to appear as a whole-file modification.
\subsection{Naming convention}
The names of the files should be in ascii, lower case, without space or numbers.
The worst thing you could do is to have two files with the same name but with a different case:
this leads to severe subversion issues on windows systems (one of the file erase the other automatically at the checkout of the repository).
Also, please avoid too long lines in the source code: respecting a maximum of 80 chars per line is good rule of thumb.
\section{Clarify and document your code}
Keep in mind that your code will be read, reused and (eventually) improved by humans.
You want them to use your code wisely, and you don't have the time to help each of them personally.
Write the documentation and the code at the same time.
Detail the how of your code, but don't forget the why.
Knowing the goal of your code will help one to detect/correct wrong behavior.
Do not hesitate to detail any specific or unusual implementation
(e.g. the use of the specific structure of a sparse matrix to gain computation time).
Similarly, document your variables: units, specific bounds, legal values and give them \textit{explicit} names (avoid \begin{tt}tmp\end{tt} or random sequence of letters).
Please refer to the articles corresponding to the code, \textit{especially} if the notations come from them.
Yet, do not overload your code by documenting the obvious. This can be avoided:\\
\begin{tt} i+=1 //increase the index value of 1\end{tt}
\section{Test a lot}
Testing is the easiest way to check the behavior and good health of your code through its evolution.
It allows to find what (and who) is responsible for a wrong behavior.
Also, it can also be of help to understand the code.
The sooner the tests are added to the project, the easier it is to debug it.
Similarly, the more automatized it is, the better.
\begin{itemize}[noitemsep,topsep=0pt,parsep=0pt,partopsep=0pt]
\item Do unit tests (verifies the behavior of the code)
\item Do regression tests (verifies that the behavior of the code do not change with the evolution of the code)
\item Do computation time tests to find bottleneck in your code (e.g. use callgrind in C++ )
\item Do memory handling tests (valgrind).
\end{itemize}
Automatize your tests using ctest or gtest\footnote{\url{http://www.ros.org/wiki/gtest}}.
\section{Bug tracking - bug report}
Eventually, you'll encounter some issues using code developed by someone else.\\
Do not hesitate to report an issue, and to signal the issue in the code with a TODO.\\
Please be explicit when you report an issue, and define the following informations:
your goal (maybe there is another/safer way to do what you're trying to do),
your OS (32 bits/64 bits), your version of ROS, the version of the packages you are using...\\
Prefer opening a ticket on the project management web application associated to the package, than contacting directly the person in charge of the code.
You might not be the only one to encounter the problem and the resulting discussion might help someone else.\\
Help the developers to reproduce the bug, e.g. by creating a branch corresponding to your problem.
%\note{voir la liste des choses à ne pas faire pour plus de détails}
\section{Feature request}
Similarly to the bug report, prefer opening tickets on the corresponding website.
You can help someone by doing this, especially when there are reasons not to implement the proposed feature.
\section{Deprecation}
As soon as there are users of your code, you have a responsibility not to pull the rug out from under them with sudden breaking changes.
Instead, use a process of deprecation, which means marking a feature or component as being no longer supported, with a schedule for its removal. Give users time to adapt, which is usually one release cycle, then do the removal.
Similarly, be thoughtful if you plan to change the ABI of your program.
%Deprecation can happen at multiple levels, including:
%
%\begin{verbatim}
%
%API features : Say you want to remove a method call from a library. First mark it as deprecated in the API documentation; with DOxgyen, use @deprecated. If the language supports it, also mark the code as being deprecated; in C/C++, use __attribute__ ((deprecated)). In the next release, note the deprecation in the ChangeList, with the future release at which you expect to remove it; if it's a widely used feature, make the deprecation notice prominent, and explain the reasoning behind it. In that future release, remove it.
%Packages : Say you want to remove a package. Mark it as deprecated in the Wiki documentation (e.g., put DEPRECATED at the top), with a note as to when you expect to remove it. Include notice of the deprecation in the ChangeList with the next (stack) release. If it's a widely used package, you should also send mail to your users giving them as much advance warning as possible.
%\end{verbatim}