forked from blair/orca
-
Notifications
You must be signed in to change notification settings - Fork 0
/
README.DEVELOPERS
545 lines (395 loc) · 20.1 KB
/
README.DEVELOPERS
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
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
-*-text-*-
If you are contributing code to the Orca project, please read this
first.
==========================
DEVELOPERS'S GUIDE TO ORCA
==========================
$LastChangedDate$
TABLE OF CONTENTS
* Participating in the community
* Getting the source
* What to read
* Directory layout
* Coding style
* Document everything
* Using page breaks
* Other conventions
* Writing log messages
* Crediting
* Patch submission guidelines
* Commit access
Participating in the community
==============================
The community exists mainly through mailing lists and a Subversion
source code repository:
Go to http://www.orcaware.com/mailman/listinfo and
* Join the "Orca-dev", "Orca-checkins", and "Orca-announce"
mailing lists. The dev list, orca-dev@orcaware.com, is where
almost all discussion takes place. All questions should go
there, though you might want to check the list archives first.
The "orca-checkins" list receives automated commit emails.
There are many ways to join the project, either by writing code, or by
testing.
To submit code, simply send your patches to orca-dev@orcaware.com.
No, wait, first read the rest of this file, _then_ start sending
patches to orca-dev@orcaware.com. :-)
Getting the source
==================
Orca uses the Subversion source control system to manage the source
code. Subversion is a CVS replacement that offers many features over
and above CVS. To get an overview of Subversion, check out
http://www.orcaware.com/svn/Subversion-Blair_Zajac.ppt
The Orca Subversion repository is located at
http://www.orcaware.com/svn/repos/orca/trunk/
with tagged releases located at
http://www.orcaware.com/svn/repos/orca/tags/
The Subversion home page is at
http://subversion.tigris.org/
You'll need a Subversion client to check out the latest Orca code and
make commits to the repository.
If you are using Windows, then you can use the Windows binaries
available at
http://subversion.tigris.org/servlets/ProjectDocumentList?folderID=91
Make sure to use the latest svn-*-setup.exe file.
If you are running Unix, then you'll either need to compile Subversion
for yourself, or if you're lucky and on a recent enough OS, then it's
probably a simple download and install. If you do need to compile it,
get the source code for Subversion at
http://subversion.tigris.org/servlets/ProjectDocumentList?folderID=260
and download the latest tar.gz. To build Subversion, read the INSTALL
file in the tar.gz.
Once you've downloaded and installed Subversion, run the following
command to check out Orca HEAD:
% svn checkout http://www.orcaware.com/svn/repos/orca/trunk/ orca
This will create a directory named 'orca' in the current working
directory.
You will then need to create the 'configure' script by running:
% cd orca
% ./autogen.sh
Running this will check if you have the proper version of autoconf on
your system and build 'configure'.
At this point, you will have the 'configure' script in the 'orca'
directory and you can configure and build Orca normally as described
in the 'INSTALL' file.
What to read
============
Before you can contribute code, you'll need to familiarize yourself
with the existing code base and interfaces.
Check out a copy of Orca (anonymously, if you don't yet have an
account with commit-access) -- so you can look at the code base.
Directory layout
================
A rough guide to the source tree:
config/
Files for the configure auto-configuration system.
contrib/
Contributed tools.
data_gatherers/
OS specific data gathering tools.
docs/
User documentation.
lib/
Perl modules and image files for Orca.
lib/Orca/
Perl modules that Orca uses.
orca/
The main Orca script and key utility scripts.
packages/
Important Perl modules used by Orca that don't come with Perl.
patches/
Patches for the SE toolkit.
Coding style
============
We're using Perl, and following the standard Perl style.
In general, be generous with parentheses even when you're sure about
the operator precedence, and be willing to add spaces and newlines to
avoid "code crunch". Don't worry too much about vertical density;
it's more important to make code readable than to fit that extra line
on the screen.
Document everything
===================
Every function, whether public or internal, must start out with a
documentation comment that describes what the function does. The
documentation should mention every parameter received by the function,
every possible return value, and (if not obvious) the conditions under
which the function could return an error. Put the parameter names in
upper case in the doc string, even when they are not upper case in the
actual declaration, so that they stand out to human readers.
Using page breaks
=================
We're using page breaks (the Ctrl-L character, ASCII 12) for section
boundaries in both code and plaintext prose files. This file is a
good example of how it's done: each section starts with a page break,
and immediately after the page break comes the title of the section.
This helps out people who use the Emacs page commands, such as
'pages-directory' and 'narrow-to-page'. Such people are not as scarce
as you might think, and if you'd like to become one of them, then add
(require 'page-ext) to your .emacs and type C-x C-p C-h sometime.
Other conventions
=================
In addition to the above standards, Orca uses these conventions:
* Use only spaces for indenting code, never tabs. Tab display
width is not standardized enough, and anyway it's easier to
manually adjust indentation that uses spaces.
* Stay within 80 columns, the width of a minimal standard display
window.
* We have a tradition of not marking files with the names of
individual authors (i.e., we don't put lines like "Author: foo"
or "@author foo" in a special position at the top of a source
file). This is to discourage territoriality -- even when a file
has only one author, we want to make sure others feel free to
make changes. People might be unnecessarily hesitant if someone
appears to have staked ownership on the file.
* There are many other unspoken conventions maintained throughout
the code, that are only noticed when someone unintentionally
fails to follow them. Just try to have a sensitive eye for the
way things are done, and when in doubt, ask.
Writing log messages
====================
Certain guidelines should be adhered to when writing log messages:
Make a log message for every change. The value of the log becomes
much less if developers cannot rely on its completeness. Even if
you've only changed comments, write a log that says "Doc fix." or
something.
Start off the log message with one line indicating the general nature
of the change. This not only helps put developers in the right frame
of mind for reading the rest of the log message, but also plays well
with the "CIA" bot that echoes the first line of each commit to
realtime forums like IRC. (See http://cia.navi.cx/ for details.)
Use full sentences, not sentence fragments. Fragments are more often
ambiguous, and it takes only a few more seconds to write out what you
mean. Fragments like "Doc fix", "New file", or "New function" are
acceptable because they are standard idioms, and all further details
should appear in the source code.
The log message should name every affected function, variable, macro,
makefile target, grammar rule, etc, including the names of symbols
that are being removed in this commit. This helps people searching
through the logs later. Don't hide names in wildcards, because the
globbed portion may be what someone searches for later. For example,
this is bad:
* twirl.c
(twirling_baton_*): Removed these obsolete structures.
(handle_parser_warning): Pass data directly to callees, instead
of storing in twirling_baton_*.
* twirl.h: Fix indentation.
Later on, when someone is trying to figure out what happened to
'twirling_baton_fast', they may not find it if they just search for
"_fast". A better entry would be:
* twirl.c
(twirling_baton_fast, twirling_baton_slow): Removed these
obsolete structures.
(handle_parser_warning): Pass data directly to callees, instead
of storing in twirling_baton_*.
* twirl.h: Fix indentation.
The wildcard is okay in the description for 'handle_parser_warning',
but only because the two structures were mentioned by full name
elsewhere in the log entry.
Note how each file gets its own entry prefixed with an "*", and the
changes within a file are grouped by symbol, with the symbols listed
in parentheses followed by a colon, followed by text describing the
change. Please adhere to this format -- not only does consistency aid
readability, it also allows software to colorize log entries
automatically.
If your change is related to a specific issue in the issue tracker,
then include a string like "issue #N" in the log message. For
example, if a patch resolves issue 1729, then the log message might
be:
Fix issue #1729: Don't crash because of a missing file.
* get_editor.c
(frobnicate_file): Check that file exists before frobnicating.
For large changes or change groups, group the log entry into
paragraphs separated by blank lines. Each paragraph should be a set
of changes that accomplishes a single goal, and each group should
start with a sentence or two summarizing the change. Truly
independent changes should be made in separate commits, of course.
One should never need the log entries to understand the current code.
If you find yourself writing a significant explanation in the log, you
should consider carefully whether your text doesn't actually belong in
a comment, alongside the code it explains. Here's an example of doing
it right:
(consume_count): If 'count' is unreasonable, return 0 and don't
advance input pointer.
And then, in 'consume_count' in 'cplus-dem.c':
while (isdigit ((unsigned char)**type))
{
count *= 10;
count += **type - '0';
/* A sanity check. Otherwise a symbol like
'_Utf390_1__1_9223372036854775807__9223372036854775'
can cause this function to return a negative value.
In this case we just consume until the end of the string. */
if (count > strlen (*type))
{
*type = save;
return 0;
}
This is why a new function, for example, needs only a log entry saying
"New Function" --- all the details should be in the source.
There are some common-sense exceptions to the need to name everything
that was changed:
* If you have made a change which requires trivial changes
throughout the rest of the program (e.g., renaming a variable),
you needn't name all the functions affected, you can just say
"All callers changed".
* If you have rewritten a file completely, the reader understands
that everything in it has changed, so your log entry may simply
give the file name, and say "Rewritten".
* If your change was only to one file, or was the same change to
multiple files, then there's no need to list their paths in the
log message (because "svn log" can show the changed paths for
that revision anyway). Only when you need to describe how the
change affected different areas in different ways is it
necessary to organize the log message by paths and symbols, as
in the examples above.
In general, there is a tension between making entries easy to find by
searching for identifiers, and wasting time or producing unreadable
entries by being exhaustive. Use your best judgment --- and be
considerate of your fellow developers. (Also, run "svn log" to see
how others have been writing their log entries.)
Crediting
=========
It is very important to record code contributions in a consistent and
parseable way. This allows us to write scripts which help us figure
out who has been actively contributing and how, so we can spot
potential new committers quickly. The Subversion project uses
human-readable but machine-parseable fields in log messages for this,
as described below.
When committing a patch written by someone else, use "Patch by: " at
the beginning of a line:
Fix issue #1729: Don't crash because of a missing file.
Patch by: J. Random <jrandom@example.com>
* subversion/libsvn_ra_ansible/get_editor.c
(frobnicate_file): Check that file exists before frobnicating.
If multiple people wrote the patch, name them all, one person per
line, making sure to start each continuation line with whitespace. If
you (the committer) were one of the people, list yourself as "me".
Thus:
Fix issue #1729: Don't crash because of a missing file.
Patch by: J. Random <jrandom@example.com>
Enrico Caruso <codingtenor@codingtenor.com>
me
* subversion/libsvn_ra_ansible/get_editor.c
(frobnicate_file): Check that file exists before frobnicating.
If someone pointed out a problem or suggested the fix, but didn't
actually write the patch, use "Suggested by: ":
Fix issue #1729: Don't crash because of a missing file.
Suggested by: J. Random <jrandom@example.com>
* subversion/libsvn_ra_ansible/get_editor.c
(frobnicate_file): Check that file exists before frobnicating.
If someone helped review the change, use "Review by:"
Fix issue #1729: Don't crash because of a missing file.
Review by: Eagle Eyes <eeyes@example.com>
* subversion/libsvn_ra_ansible/get_editor.c
(frobnicate_file): Check that file exists before frobnicating.
(As with "Patch by: ", you can name multiple people in "Review by: "
or "Suggested by: " via whitespace-prefixed continuation lines.)
Multiple fields in the same log message are fine, for example:
Fix issue #1729: Don't crash because of a missing file.
Patch by: J. Random <jrandom@example.com>
Enrico Caruso <codingtenor@codingtenor.com>
me
Suggested by: J. Random <jrandom@example.com>
Review by: Eagle Eyes <eeyes@example.com>
* subversion/libsvn_ra_ansible/get_editor.c
(frobnicate_file): Check that file exists before frobnicating.
To give further details about a contribution, use a parenthetical
aside immediately after that field, for example:
Fix issue #1729: Don't crash because of a missing file.
Patch by: J. Random <jrandom@example.com>
(Tweaked by me.)
* subversion/libsvn_ra_ansible/get_editor.c
(frobnicate_file): Check that file exists before frobnicating.
It is understood that a parenthetical aside immediately following a
field applies to that field, and that "me" refers to person who
committed this revision. You don't have to write "me", you can use
your name instead, if you're not tired of typing it. Also, although
the examples above use full name and email address, you can use a
committer's username to refer to that committer from any field. For
example, "Philip Martin <philip@codematters.co.uk>" and "philip" would
be equivalent. See the leftmost column of the COMMITTERS file for
canonical usernames.
Currently, these three fields
Patch by:
Suggested by:
Review by:
are the only officially-supported crediting fields (where "supported"
means scripts know to look for them), and they are widely used in
Subversion log messages. Future fields would probably also be of the
form "VERB by: ", and from time to time someone may use a field that
sounds official but really isn't -- there are a few instances of
"Reported by: ", for example. These are okay, but try to use an
official field, or a parenthetical aside, in preference to making up
your own field. Also, don't use "Reported by: " when the reporter is
already recorded in an issue; instead, just refer to the issue.
Look over Orca's existing log messages to see how to use these fields
in practice. This command from the top of your trunk working copy
will help:
wget http://svn.collab.net/repos/svn/trunk/contrib/client-side/search-svnlog.pl
svn log | ./search-svnlog.pl "(Patch|Review|Suggested) by: "
Note that the "Approved by: " field seen in some commits is totally
unrelated to these crediting fields (and is rarely parsed by scripts).
It is simply the standard syntax for indicating either who approved a
partial committer's commit outside their usual area, or (in the case
of merges to release branches) who voted for the change to be merged.
Patch submission guidelines
===========================
Mail patches to 'orca-dev@orcaware.com', with a subject line that
contains the word "PATCH" in all uppercase, for example
Subject: [PATCH] fix for Orca images
A patch submission should contain one logical change; please don't mix
N unrelated changes in one submission -- send N separate emails
instead.
The email message should start off with a log message, as described in
"Writing log messages" above. The patch itself should be in unified
diff format (e.g., with "svn diff"), preferably inserted directly into
the body of your message (rather than MIME-attached, uuencoded, or
otherwise opaqified). If your mailer wraps long lines, then you will
need to attach your patch. Please ensure the MIME type of the
attachment is text/plain (some mailers allow you to set the MIME type;
for some others, you might have to use a .txt extension on your patch
file). Do not compress or otherwise encode the attached patch.
If the patch implements a new feature, make sure to describe the
feature completely in your mail; if the patch fixes a bug, describe
the bug in detail and give a reproduction recipe. An exception to
these guidelines is when the patch addresses a specific issue in the
issues database -- in that case, just make sure to refer to the issue
number in your log message, as described in "Writing log messages".
It is normal for patches to undergo several rounds of feedback and
change before being applied. Don't be discouraged if your patch is
not accepted immediately -- it doesn't mean you goofed, it just means
that there are a *lot* of eyes looking at every code submission, and
it's a rare patch that doesn't have at least a little room for
improvement. After reading people's responses to your patch, make the
appropriate changes and resubmit, wait for the next round of feedback,
and lather, rinse, repeat, until some committer applies it.
If you don't get a response for a while, and don't see the patch
applied, it may just mean that people are really busy. Go ahead and
repost, and don't hesitate to point out that you're still waiting for
a response. One way to think of it is that patch management is highly
parallizable, and we need you to shoulder your share of the management
as well as the coding. Every patch needs someone to shepherd it
through the process, and the person best qualified to do that is the
original submitter.
Commit access
=============
After someone has successfully contributed a few non-trivial patches,
some committer, usually whoever has reviewed and applied the most
patches from that contributor, proposes them for commit access. This
proposal is sent only to the other full committers -- the ensuing
discussion is private, so that everyone can feel comfortable speaking
their minds. Assuming there are no objections, the contributor is
granted commit access. The decision is made by consensus; there are
no formal rules governing the procedure, though generally if someone
strongly objects the access is not offered, or is offered on a
provisional basis.
The criteria for commit access are that the person's patches adhere to
the guidelines in this file, adhere to all the usual unquantifiable
rules of coding (code should readable, robust, maintainable, etc), and
that the person respects the "Hippocratic Principle": first, do no
harm. In other words, what is significant is not the size or quantity
of patches submitted, but the degree of care shown in avoiding bugs
and minimizing unnecessary impact on the rest of the code. Many
committers are people who have not made major code contributions, but
rather lots of small, clean fixes, each of which was an unambiguous
improvement to the code.
See the COMMITTERS file for a complete list of committers.