3 RT::StyleGuide - RT Style Guide
7 This file is somewhat out of date; L<hacking> takes precedence over it.
11 All code and documentation that is submitted to be included in the RT
12 distribution should follow the style in this document. This is not to
13 try to stifle your creativity, but to make life easier for everybody who
14 has to work with your code, and to aid those who are not quite sure how
17 These conventions below apply to perl modules, web programs, and
18 command-line programs, specifically, but also might apply to some
19 degree to any Perl code written for use in RT.
21 Note that these are all guidelines, not unbreakable rules. If you have
22 a really good need to break one of the rules herein, however, then it is
23 best to ask on the B<rt-devel> mailing list first.
25 Note that with much of this document, it is not so much the Right Way as
26 it is Our Way. We need to have conventions in order to make life easier
27 for everyone. So don't gripe, and just follow it, because you didn't
28 get a good grade in "Plays Well With Others" in kindergarten and you
29 want to make up for it now.
31 If you have any questions, please ask us on the B<rt-devel> mailing list:
33 http://www.bestpractical.com/rt/lists.html
35 We don't always follow this guide. We are making changes throughout
36 our code to be in line with it. But just because we didn't do
37 it yet, that is no excuse. Do it anyway. :-)
39 This document is subject to change at the whims of the core RT team.
40 We hope to add any significant changes at the bottom of the document.
43 =head1 CODING PRINCIPLES
47 We code everything to Perl 5.10.1 or higher.
51 All modules will be documented using the POD examples in the module
52 boilerplate. The function, purpose, use of the module will be
53 explained, and each public API will be documented with name,
54 description, inputs, outputs, side effects, etc.
56 If an array or hash reference is returned, document the size of the
57 array (including what each element is, as appropriate) and name each key
58 in the hash. For complex data structures, map out the structure as
59 appropriate (e.g., name each field returned for each column from a DB
60 call; yes, this means you shouldn't use "SELECT *", which you shouldn't
63 Also document what kind of data returned values are. Is it an integer,
64 a block of HTML, a boolean?
66 All command-line program options will be documented using the
67 boilerplate code for command-line programs, which doesn't yet exist.
68 Each available function, switch, etc. should be documented, along
69 with a statement of function, purpose, use of the program. Do not
70 use the same options as another program, for a different purpose.
72 All web templates should be documented with a statement of function,
73 purpose, and use in a mason comment block.
75 Any external documents, and documentation for command-line programs and
76 modules, should be written in POD, where appropriate. From there, they
77 can be translated to many formats with the various pod2* translators.
78 Read the perlpod manpage before writing any POD, because although POD is
79 not difficult, it is not what most people are used to. It is not a
80 regular markup language; it is just a way to make easy documentation
81 for translating to other formats. Read, and understand, the perlpod
82 manpage, and ask us or someone else who knows if you have any questions.
87 Our distribution versions use tuples, where the first number is the
88 major revision, the second number is the version, and third
89 number is the subversion. Odd-numbered versions are development
92 1.0.0 First release of RT 1
93 1.0.1 Second release of RT 1.0
95 1.1.0 First development release of RT 1.2 (or 2.0)
96 2.0.0 First release of RT 2
98 Versions may end in "rc" and a number if they are release candidates:
100 2.0.0rc1 First release candiate for real 2.0.0
105 All code should be self-documenting as much as possible. Only include
106 necessary comments. Use names like "$ticket_count", so you don't need to
112 Include any comments that are, or might be, necessary in order for
113 someone else to understand the code. Sometimes a simple one-line
114 comment is good to explain what the purpose of the following code is
115 for. Sometimes each line needs to be commented because of a complex
116 algorithm. Read Kernighan & Pike's I<Practice of Programming> about
117 commenting. Good stuff, Maynard.
120 =head2 Warnings and Strict
122 All code must compile and run cleanly with "use strict" enabled and the
123 perl "-w" (warnings) option on. If you must do something that -w or
124 strict complains about, there are workarounds, but the chances that you
125 really need to do it that way are remote.
127 =head2 Lexical Variables
129 Use only lexical variables, except for special global variables
130 ($VERSION, %ENV, @ISA, $!, etc.) or very special circumstances (see
131 %HTML::Mason::Commands::session ). Global variables
132 for regular use are never appropriate. When necessary, "declare"
133 globals with "use vars" or "our()".
135 A lexical variable is created with my(). A global variable is
136 pre-existing (if it is a special variable), or it pops into existence
137 when it is used. local() is used to tell perl to assign a temporary
138 value to a variable. This should only be used with special variables,
139 like $/, or in special circumstances. If you must assign to any global
140 variable, consider whether or not you should use local().
142 local() may also be used on elements of arrays and hashes, though there
143 is seldom a need to do it, and you shouldn't.
146 =head2 Pass by Reference
148 Arrays and hashes should be passed to and from functions by reference
149 only. Note that a list and an array are NOT the same thing. This
152 return($user, $form, $constants);
154 An exception might be a temporary array of discrete arguments:
156 my @return = ($user, $form);
157 push @return, $constants if $flag;
160 Although, usually, this is better (faster, easier to read, etc.):
163 return($user, $form, $constants);
165 return($user, $form);
168 We need to talk about Class::ReturnValue here.
171 =head2 Method parameters
173 If a method takes exactly one mandatory argument, the argument should be
174 passed in a straightforward manner:
179 In all other cases, the method needs to take named parameters, usually
180 using a C<%args> hash to store them:
185 Description => undef,
189 You may specify defaults to those named parameters instead of using
190 C<undef> above, as long as it is documented as such.
192 It is worth noting that the existing RT codebase had not followed this
193 style perfectly; we are trying to fix it without breaking existing APIs.
197 Modules should provide test code, with documentation on how to use
198 it. Test::More makes it easy to create tests. Any code you write
199 should have a testsuite. Any code you alter should have a test
200 suite. If a patch comes in without tests, there is something wrong.
202 When altering code, you must run the test harness before submitting a
203 patch or committing code to the repository.
205 "make test" will run the test suite.
209 Always report errors using $RT::Logger. It's a Log::Dispatch object.
210 Unlike message meant for the user, log messages are not to be
213 There are several different levels ($RT::Logger methods) of logging:
219 Used for messages only needed during system debugging.
223 Should be used to describe "system-critical" events which aren't errors.
224 Examples: creating users, deleting users, creating tickets, creating queues,
225 sending email (message id, time, recipients), recieving mail, changing
226 passwords, changing access control, superuser logins)
230 Used for RT-generated failures during execution.
234 Should be used for messages when an action can not be completed due to some
235 error condition beyond our control.
239 In the web UI and modules, never print directly to STDERR. Do not print
240 directly to STDOUT, unless you need to print directly to the user's console.
242 In command-line programs, feel free to print to STDERR and STDOUT as
243 needed for direct console communication. But for actual error reporting,
249 Always check return values from system calls, including open(),
250 close(), mkdir(), or anything else that talks directly to the system.
251 Perl built-in system calls return the error in $!; some functions in
252 modules might return an error in $@ or some other way, so read the module's
253 documentation if you don't know. Always do something, even if it is
254 just calling $RT::Logger->warning(), when the return value is not what you'd expect.
260 Much of the style section is taken from the perlsyle manpage. We make
261 some changes to it here, but it wouldn't be a bad idea to read that
268 =item function vs. sub(routine) vs. method
270 Just because it is the Perl Way (not necessarily right for all
271 languages, but the documented terminology in the perl documentation),
272 "method" should be used only to refer to a subroutine that are object
273 methods or class methods; that is, these are functions that are used
274 with OOP that always take either an object or a class as the first
275 argument. Regular subroutines, ones that are not object or class
276 methods, are functions. Class methods that create and return an object
277 are optionally called constructors.
281 "users" are normally users of RT, the ones hitting the site; if using
282 it in any other context, specify.
283 "system users" are user
284 names on the operating system. "database users" are the user names in
285 the database server. None of these needs to be capitalized.
292 Don't use single-character variables, except as iterator variables.
294 Don't use two-character variables just to spite us over the above rule.
296 Constants are in all caps; these are variables whose value will I<never>
297 change during the course of the program.
299 $Minimum = 10; # wrong
300 $MAXIMUM = 50; # right
302 Other variables are lowercase, with underscores separating the words.
303 They words used should, in general, form a noun (usually singular),
304 unless the variable is a flag used to denote some action that should be
305 taken, in which case they should be verbs (or gerunds, as appropriate)
306 describing that action.
308 $thisVar = 'foo'; # wrong
309 $this_var = 'foo'; # right
310 $work_hard = 1; # right, verb, boolean flag
311 $running_fast = 0; # right, gerund, boolean flag
313 Arrays and hashes should be plural nouns, whether as regular arrays and
314 hashes or array and hash references. Do not name references with "ref"
315 or the data type in the name.
317 @stories = (1, 2, 3); # right
318 $comment_ref = [4, 5, 6]; # wrong
319 $comments = [4, 5, 6]; # right
320 $comment = $comments->[0]; # right
322 Make the name descriptive. Don't use variables like "$sc" when you
323 could call it "$story_count". See L<"Comments">.
325 There are several variables in RT that are used throughout the code,
326 that you should use in your code. Do not use these variable names for
327 anything other than how they are normally used, and do not use any
328 other variable names in their place. Some of these are:
330 $self # first named argument in object method
332 Subroutines (except for special cases, like AUTOLOAD and simple accessors)
333 begin with a verb, with words following to complete the action. Accessors
334 don't start with "Get" if they're just the name of the attribute.
336 Accessors which return an object should end with the suffix Obj.
338 This section needs clarification for RT.
340 Words begin with a capital letter. They
341 should as clearly as possible describe the activity to be peformed, and
342 the data to be returned.
350 Subroutines beginning with C<_> are special: they are not to be used
351 outside the current object. There is not to be enforced by the code
352 itself, but by someone very big and very scary.
354 For large for() loops, do not use $_, but name the variable.
355 Do not use $_ (or assume it) except for when it is absolutely
356 clear what is going on, or when it is required (such as with
360 print; # OK; everyone knows this one
361 print uc; # wrong; few people know this
362 print uc $_; # better
365 Note that the special variable C<_> I<should> be used when possible.
366 It is a placeholder that can be passed to stat() and the file test
367 operators, that saves perl a trip to re-stat the file. In the
368 example below, using C<$file> over for each file test, instead of
369 C<_> for subsequent uses, is a performance hit. You should be
370 careful that the last-tested file is what you think it is, though.
372 if (-d $file) { # $file is a directory
374 } elsif (-l _) { # $file is a symlink
378 Package names begin with a capital letter in each word, followed by
379 lower case letters (for the most part). Multiple words should be StudlyCapped.
382 RT::Database::MySQL # proper name
383 RT::Display::Provider # good
384 RT::CustomField # not so good, but OK
386 Plugin modules should begin with "RT::Extension::", followed by the name
389 =head1 Code formatting
391 When in doubt, use perltidy; RT includes a F<.perltidyrc>.
393 =head2 Indents and Blank Space
395 All indents should be four spaces; hard tabs are forbidden.
397 No space before a semicolon that closes a statement.
402 Line up corresponding items vertically.
408 open(FILE, $fh) or die $!;
409 open(FILE2, $fh2) or die $!;
411 $rot13 =~ tr[abcedfghijklmnopqrstuvwxyz]
412 [nopqrstuvwxyzabcdefghijklm];
414 # note we use a-mn-z instead of a-z,
419 Put blank lines between groups of code that do different things. Put
420 blank lines after your variable declarations. Put a blank line before a
421 final return() statement. Put a blank line following a block (and
422 before, with the exception of comment lines).
426 # this is my function!
429 my $obj = new Constructor;
443 For control structures, there is a space between the keyword and opening
444 parenthesis. For functions, there is not.
452 Be careful about list vs. scalar context with parentheses!
454 my @array = ('a', 'b', 'c');
455 my($first_element) = @array; # a
456 my($first_element) = ('a', 'b', 'c'); # a
457 my $element_count = @array; # 3
458 my $last_element = ('a', 'b', 'c'); # c
460 Always include parentheses after functions, even if there are no arguments.
461 There are some exceptions, such as list operators (like print) and unary
462 operators (like undef, delete, uc).
464 There is no space inside the parentheses, unless it is needed for
467 for ( map { [ $_, 1 ] } @list ) # OK
468 for ( @list ) # not really OK, not horrible
470 On multi-line expressions, match up the closing parenthesis with either
471 the opening statement, or the opening parenthesis, whichever works best.
479 if ($foo && $bar && $baz
484 Whether or not there is space following a closing parenthesis is
485 dependent on what it is that follows.
487 print foo(@bar), baz(@buz) if $xyzzy;
489 Note also that parentheses around single-statement control expressions,
490 as in C<if $xyzzy>, are optional (and discouraged) C<if> it is I<absolutely>
491 clear -- to a programmer -- what is going on. There is absolutely no
492 need for parentheses around C<$xyzzy> above, so leaving them out enhances
493 readability. Use your best discretion. Better to include them, if
494 there is any question.
496 The same essentially goes for perl's built-in functions, when there is
497 nothing confusing about what is going on (for example, there is only one
498 function call in the statement, or the function call is separated by a
499 flow control operator). User-supplied functions must always include
502 print 1, 2, 3; # good
503 delete $hash{key} if isAnon($uid); # good
506 However, if there is any possible confusion at all, then include the
507 parentheses. Remember the words of Larry Wall in the perlstyle manpage:
509 When in doubt, parenthesize. At the very least it will
510 let some poor schmuck bounce on the % key in vi.
512 Even if you aren't in doubt, consider the mental welfare
513 of the person who has to maintain the code after you, and
514 who will probably put parens in the wrong place.
516 So leave them out when it is absoutely clear to a programmer, but if
517 there is any question, leave them in.
522 (This is about control braces, not hash/data structure braces.)
524 There is always a space befor the opening brace.
526 while (<$fh>){ # wrong
527 while (<$fh>) { # right
529 A one-line block may be put on one line, and the semicolon may be
532 for (@list) { print }
534 Otherwise, finish each statement with a semicolon, put the keyword and
535 opening curly on the first line, and the ending curly lined up with the
543 Generally, we prefer "cuddled elses":
553 Put space around most operators. The primary exception is the for
554 aesthetics; e.g., sometimes the space around "**" is ommitted,
555 and there is never a space before a ",", but always after.
557 print $x , $y; # wrong
558 print $x, $y; # right
563 Note that "&&" and "||" have a higher precedence than "and" and "or".
564 Other than that, they are exactly the same. It is best to use the lower
565 precedence version for control, and the higher for testing/returning
568 $bool = $flag1 or $flag2; # WRONG (doesn't work)
569 $value = $foo || $bar; # right
570 open(FILE, $file) or die $!;
572 $true = foo($bar) && baz($buz);
573 foo($bar) and baz($buz);
575 Note that "and" is seldom ever used, because the statement above is
576 better written using "if":
578 baz($buz) if foo($bar);
580 Most of the time, the confusion between and/&&, or/|| can be alleviated
581 by using parentheses. If you want to leave off the parentheses then you
582 I<must> use the proper operator. But if you use parentheses -- and
583 normally, you should, if there is any question at all -- then it doesn't
584 matter which you use. Use whichever is most readable and aesthetically
585 pleasing to you at the time, and be consistent within your block of code.
587 Break long lines AFTER operators, except for ".", "and", "or", "&&", "||".
588 Try to keep the two parts to a binary operator (an operator that
589 has two operands) together when possible.
591 print "foo" . "bar" . "baz" .
594 print "foo" . "bar" . "baz"
597 print $foo unless $x == 3 && $y ==
598 4 && $z == 5; # wrong
600 print $foo unless $x == 3 && $y == 4
606 Put space around a complex subscript inside the brackets or braces.
608 $foo{$bar{baz}{buz}}; # OK
609 $foo{ $bar{baz}{buz} }; # better
611 In general, use single-quotes around literals, and double-quotes
612 when the text needs to be interpolated.
614 It is OK to omit quotes around names in braces and when using
615 the => operator, but be careful not to use a name that doubles as
616 a function; in that case, quote.
618 $what{'time'}{it}{is} = time();
620 When making compound statements, put the primary action first.
622 open(FILE, $fh) or die $!; # right
623 die $! unless open(FILE, $fh); # wrong
625 print "Starting\n" if $verbose; # right
626 $verbose && print "Starting\n"; # wrong
629 Use here-docs instead of repeated print statements.
632 This is a whole bunch of text.
633 I like it. I don't need to worry about messing
634 with lots of print statements and lining them up.
637 Just remember that unless you put single quotes around your here-doc
638 token (<<'EOT'), the text will be interpolated, so escape any "$" or "@"
641 =head1 INTERNATIONALIZATION
644 =head2 String extraction styleguide
650 Templates should use the /l filtering component to call the localisation
655 Should become <&|/l&>Foo!</&>
657 All newlines should be removed from localized strings, to make it easy to
658 grep the codebase for strings to be localized
664 Should become <&|/l&>Foo Bar Baz</&>
667 Variable subsititutions should be moved to Locale::MakeText format
669 The string Hello, <%$name %>
671 should become <&|/l, $name &>Hello, [_1]</&>
674 Multiple variables work just like single variables
676 The string You found <%$num%> tickets in queue <%$queue%>
678 should become <&|/l, $num, $queue &>You found [_1] tickets in queue [_2]</&>
680 When subcomponents are called in the middle of a phrase, they need to be escaped
683 The string <input type="submit" value="New ticket in"> <& /Elements/SelectNewTicketQueue&>
685 should become <&|/l, $m->scomp('/Elements/SelectNewTicketQueue')&><input type="submit" value="New ticket in"> [_1]</&>
690 The string <& /Widgets/TitleBoxStart, width=> "40%", titleright => "RT $RT::VERSION for RT->Config->Get('rtname')", title => 'Login' &>
692 should become <& /Widgets/TitleBoxStart,
694 titleright => loc("RT [_1] for [_2]",$RT::VERSION, RT->Config->Get('rtname')),
695 title => loc('Login'),
702 Within RT's core code, every module has a localization handle available through the 'loc' method:
704 The code return ( $id, "Queue created" );
706 should become return ( $id, $self->loc("Queue created") );
708 When returning or localizing a single string, the "extra" set of parenthesis () should be omitted.
710 The code return ("Subject changed to ". $self->Data );
712 should become return $self->loc( "Subject changed to [_1]", $self->Data );
715 It is important not to localize the names of rights or statuses within RT's core, as there is logic that depends on them as string identifiers. The proper place to localize these values is when they're presented for display in the web or commandline interfaces.
720 =head1 CODING PRCEDURE
722 This is for new programs, modules, specific APIs, or anything else.
726 =item Present idea to rt-devel
728 We may know of a better way to approach the problem, or know of an
729 existing way to deal with it, or know someone else is working on it.
730 This is mostly informal, but a fairly complete explanation for the need
731 and use of the code should be provided.
734 =item Present complete specs to rt-devel
736 The complete proposed API should be submitted for
737 approval and discussion. For web and command-line programs, present the
738 functionality and interface (op codes, command-lin switches, etc.).
740 The best way to do this is to take the documentation portion of the
741 boilerplate and fill it in. You can make changes later if necessary,
742 but fill it in as much as you can.
746 =item Prepare for code review
748 When you are done, the code will undergo a code review by a member of
749 the core team, or someone picked by the core team. This is not to
750 belittle you (that's just a nice side effect), it is to make sure that
751 you understand your code, that we understand your code, that it won't
752 break other code, that it follows the documentation and existing
753 proposal. It is to check for possible optimizations or better ways of
756 Note that all code is expected to follow the coding principles and style
757 guide contained in this document.
762 After the code is done (possibly going through multiple code reviews),
763 if you do not have repository access, submit it to rt-bugs@fsck.com as a
764 unified diff. From that point on, it'll be handled by someone with
770 =head1 BUG REPORTS, PATCHES
772 Use rt-bugs@bestpractical.com for I<any> bug that is not being fixed
773 immediately. If it is not in RT, there is a good chance it will not be
776 Send patches to rt-bugs@bestpractical.com, too. Use C<diff -u> for
781 RT uses a convention to denote the foreign key status in its tables.
782 The rule of thumb is:
786 =item When it references to another table, always use the table name
788 For example, the C<Template> field in the C<Scrips> table refers to
789 the C<Id> of the same-named C<Template> table.
791 =item Otherwise, always use the C<Id> suffix
793 For example, the C<ObjectId> field in the C<ACL> table can refer
794 to any object, so it has the C<Id> suffix.
798 There are some legacy fields that did not follow this rule, namely
799 C<ACL.PrincipalId>, C<GroupMembers.GroupId> and C<Attachments.TransactionId>,
800 but new tables are expected to be consistent.
803 =head1 EXTENDING RT CLASSES
805 =head2 The Overlay mechanism
807 RT's classes allow "overlay" methods to be placed into files named Filename_Vendor.pm and Filename_Local.pm
808 _Vendor is for 3rd-party vendor add-ons, while _Local is for site-local customizations.
810 These overlay files can contain new subs or subs to replace existing subs in this module.
812 Each of these files should begin with the line
814 no warnings qw(redefine);
816 so that perl does not kick and scream when you redefine a subroutine or variable in your overlay.
820 Talk about DBIx::SearchBuilder
824 cascading style sheets
826 Talk about adding a new translation
828 Talk more about logging