3 RT::StyleGuide - RT Style Guide
7 All code and documentation that is submitted to be included in the RT
8 distribution should follow the style in this document. This is not to
9 try to stifle your creativity, but to make life easier for everybody who
10 has to work with your code, and to aid those who are not quite sure how
13 These conventions below apply to perl modules, web programs, and
14 command-line programs, specifically, but also might apply to some
15 degree to any Perl code written for use in RT.
17 Note that these are all guidelines, not unbreakable rules. If you have
18 a really good need to break one of the rules herein, however, then it is
19 best to ask on the B<rt-devel> mailing list first.
21 Note that with much of this document, it is not so much the Right Way as
22 it is Our Way. We need to have conventions in order to make life easier
23 for everyone. So don't gripe, and just follow it, because you didn't
24 get a good grade in "Plays Well With Others" in kindergarten and you
25 want to make up for it now.
27 If you have any questions, please ask us on the B<rt-devel> mailing list:
29 http://www.bestpractical.com/rt/lists.html
31 We don't always follow this guide. We are making changes throughout
32 our code to be in line with it. But just because we didn't do
33 it yet, that is no excuse. Do it anyway. :-)
35 This document is subject to change at the whims of the core RT team.
36 We hope to add any significant changes at the bottom of the document.
39 =head1 CODING PRINCIPLES
43 We code everything to perl 5.8.3 or higher. Complete unicode support
44 requires bugfixes found in 5.8.3.
48 All modules will be documented using the POD examples in the module
49 boilerplate. The function, purpose, use of the module will be
50 explained, and each public API will be documented with name,
51 description, inputs, outputs, side effects, etc.
53 If an array or hash reference is returned, document the size of the
54 array (including what each element is, as appropriate) and name each key
55 in the hash. For complex data structures, map out the structure as
56 appropriate (e.g., name each field returned for each column from a DB
57 call; yes, this means you shouldn't use "SELECT *", which you shouldn't
60 Also document what kind of data returned values are. Is it an integer,
61 a block of HTML, a boolean?
63 All command-line program options will be documented using the
64 boilerplate code for command-line programs, which doesn't yet exist.
65 Each available function, switch, etc. should be documented, along
66 with a statement of function, purpose, use of the program. Do not
67 use the same options as another program, for a different purpose.
69 All web templates should be documented with a statement of function,
70 purpose, and use in a mason comment block.
72 Any external documents, and documentation for command-line programs and
73 modules, should be written in POD, where appropriate. From there, they
74 can be translated to many formats with the various pod2* translators.
75 Read the perlpod manpage before writing any POD, because although POD is
76 not difficult, it is not what most people are used to. It is not a
77 regular markup language; it is just a way to make easy documentation
78 for translating to other formats. Read, and understand, the perlpod
79 manpage, and ask us or someone else who knows if you have any questions.
84 Our distribution versions use tuples, where the first number is the
85 major revision, the second number is the version, and third
86 number is the subversion. Odd-numbered versions are development
89 1.0.0 First release of RT 1
90 1.0.1 Second release of RT 1.0
92 1.1.0 First development release of RT 1.2 (or 2.0)
93 2.0.0 First release of RT 2
95 Versions can be modified with a hyphen followed by some text, for
96 special versions, or to give extra information. Examples:
98 2.0.0-pre1 Notes that this is not final, but preview
100 In perl 5.6.0, you can have versions like C<v2.0.0>, but this is not
101 allowed in previous versions of perl. So to convert a tuple version
102 string to a string to use with $VERSION, use a regular integer for
103 the revision, and three digits for version and subversion. Examples:
108 This way, perl can use the version strings in greater-than and
109 less-than comparisons.
114 All code should be self-documenting as much as possible. Only include
115 necessary comments. Use names like "$ticket_count", so you don't need to
121 Include any comments that are, or might be, necessary in order for
122 someone else to understand the code. Sometimes a simple one-line
123 comment is good to explain what the purpose of the following code is
124 for. Sometimes each line needs to be commented because of a complex
125 algorithm. Read Kernighan & Pike's I<Practice of Programming> about
126 commenting. Good stuff, Maynard.
129 =head2 Warnings and Strict
131 All code must compile and run cleanly with "use strict" enabled and the
132 perl "-w" (warnings) option on. If you must do something that -w or
133 strict complains about, there are workarounds, but the chances that you
134 really need to do it that way are remote.
136 =head2 Lexical Variables
138 Use only lexical variables, except for special global variables
139 ($VERSION, %ENV, @ISA, $!, etc.) or very special circumstances (see
140 %HTML::Mason::Commands::session ). Global variables
141 for regular use are never appropriate. When necessary, "declare"
142 globals with "use vars" or "our()".
144 A lexical variable is created with my(). A global variable is
145 pre-existing (if it is a special variable), or it pops into existence
146 when it is used. local() is used to tell perl to assign a temporary
147 value to a variable. This should only be used with special variables,
148 like $/, or in special circumstances. If you must assign to any global
149 variable, consider whether or not you should use local().
151 local() may also be used on elements of arrays and hashes, though there
152 is seldom a need to do it, and you shouldn't.
157 Do not export anything from a module by default. Feel free to put
158 anything you want to in @EXPORT_OK, so users of your modules can
159 explicitly ask for symbols (e.g., "use Something::Something qw(getFoo
160 setFoo)"), but do not export them by default.
163 =head2 Pass by Reference
165 Arrays and hashes should be passed to and from functions by reference
166 only. Note that a list and an array are NOT the same thing. This
169 return($user, $form, $constants);
171 An exception might be a temporary array of discrete arguments:
173 my @return = ($user, $form);
174 push @return, $constants if $flag;
177 Although, usually, this is better (faster, easier to read, etc.):
180 return($user, $form, $constants);
182 return($user, $form);
185 We need to talk about Class::ReturnValue here.
188 =head2 Garbage Collection
190 Perl does pretty good garbage collection for you. It will automatically
191 clean up lexical variables that have gone out of scope and objects whose
192 references have gone away. Normally you don't need to worry about
193 cleaning up after yourself, if using lexicals.
195 However, some glue code, code compiled in C and linked to Perl, might
196 not automatically clean up for you. In such cases, clean up for
197 yourself. If there is a method in that glue to dispose or destruct,
198 then use it as appropriate.
200 Also, if you have a long-running function that has a large data
201 structure in it, it is polite to free up the memory as soon as you are
202 done with it, if possible.
204 my $huge_data_structure = get_huge_data_structure();
205 do_something_with($huge_data_structure);
206 undef $huge_data_structure;
210 All object classes must provide a DESTROY method. If it won't do
211 anything, provide it anyway:
217 =head2 die() and exit()
219 Don't do it. Do not die() or exit() from a web template or module. Do
220 not call C<kill 9, $$>. Don't do it.
222 In command-line programs, do as you please.
227 Do not use @_. Use shift. shift may take more lines, but Jesse thinks it
228 leads to cleaner code.
230 my $var = shift; # right
231 my($var) = @_; # ick. no
232 sub foo { uc $_[0] } # icky. sometimes ok.
235 my($var1, $var2) = (shift, shift); # Um, no.
237 my $var1 = shift; # right
240 =head2 Method parameters
242 If a method takes exactly one mandatory argument, the argument should be
243 passed in a straightforward manner:
248 In all other cases, the method needs to take named parameters, usually
249 using a C<%args> hash to store them:
252 my %args = ( Name => undef,
253 Description => undef,
256 You may specify defaults to those named parameters instead of using
257 C<undef> above, as long as it is documented as such.
259 It is worth noting that the existing RT codebase had not followed this
260 style perfectly; we are trying to fix it without breaking exsiting APIs.
264 Modules should provide test code, with documentation on how to use
265 it. Test::Inline allows tests to be embedded in code. Test::More makes it
266 easy to create tests. Any code you write should have a testsuite.
267 Any code you alter should have a test suite. If a patch comes in without
268 tests, there is something wrong.
270 When altering code, you must run the test harness before submitting a patch
271 or committing code to the repository.
273 "make regression" will extract inline tests, blow away the system database
274 and run the test suite.
276 "make regression-quiet" will do all that and not print the "ok" lines.
282 Always report errors using $RT::Logger. It's a Log::Dispatch object.
283 Unlike message meant for the user, log messages are not to be
286 There are several different levels ($RT::Logger methods) of logging:
292 Used for messages only needed during system debugging.
296 Should be used to describe "system-critical" events which aren't errors.
297 Examples: creating users, deleting users, creating tickets, creating queues,
298 sending email (message id, time, recipients), recieving mail, changing
299 passwords, changing access control, superuser logins)
303 Used for RT-generated failures during execution.
307 Should be used for messages when an action can not be completed due to some
308 error condition beyond our control.
312 In the web UI and modules, never print directly to STDERR. Do not print
313 directly to STDOUT, unless you need to print directly to the user's console.
315 In command-line programs, feel free to print to STDERR and STDOUT as
316 needed for direct console communication. But for actual error reporting,
322 Always check return values from system calls, including open(),
323 close(), mkdir(), or anything else that talks directly to the system.
324 Perl built-in system calls return the error in $!; some functions in
325 modules might return an error in $@ or some other way, so read the module's
326 documentation if you don't know. Always do something, even if it is
327 just calling $RT::Logger->warning(), when the return value is not what you'd expect.
333 Much of the style section is taken from the perlsyle manpage. We make
334 some changes to it here, but it wouldn't be a bad idea to read that
343 "RT" is the name of the project. "RT" is, optionally, the
344 specific name for the actual file distribution. That's it.
346 While we sometimes use "RT2" or "RT3", that's shortand that's really
347 not recommended. The name of the project is "RT".
349 To specify a major version, use "RT 3.0".
350 To specify a specific release, use "RT 3.0.12"
352 =item function vs. sub(routine) vs. method
354 Just because it is the Perl Way (not necessarily right for all
355 languages, but the documented terminology in the perl documentation),
356 "method" should be used only to refer to a subroutine that are object
357 methods or class methods; that is, these are functions that are used
358 with OOP that always take either an object or a class as the first
359 argument. Regular subroutines, ones that are not object or class
360 methods, are functions. Class methods that create and return an object
361 are optionally called constructors.
365 "users" are normally users of RT, the ones hitting the site; if using
366 it in any other context, specify.
367 "system users" are user
368 names on the operating system. "database users" are the user names in
369 the database server. None of these needs to be capitalized.
376 Don't use single-character variables, except as iterator variables.
378 Don't use two-character variables just to spite us over the above rule.
380 Constants are in all caps; these are variables whose value will I<never>
381 change during the course of the program.
383 $Minimum = 10; # wrong
384 $MAXIMUM = 50; # right
386 Other variables are lowercase, with underscores separating the words.
387 They words used should, in general, form a noun (usually singular),
388 unless the variable is a flag used to denote some action that should be
389 taken, in which case they should be verbs (or gerunds, as appropriate)
390 describing that action.
392 $thisVar = 'foo'; # wrong
393 $this_var = 'foo'; # right
394 $work_hard = 1; # right, verb, boolean flag
395 $running_fast = 0; # right, gerund, boolean flag
397 Arrays and hashes should be plural nouns, whether as regular arrays and
398 hashes or array and hash references. Do not name references with "ref"
399 or the data type in the name.
401 @stories = (1, 2, 3); # right
402 $comment_ref = [4, 5, 6]; # wrong
403 $comments = [4, 5, 6]; # right
404 $comment = $comments->[0]; # right
406 Make the name descriptive. Don't use variables like "$sc" when you
407 could call it "$story_count". See L<"Comments">.
409 There are several variables in RT that are used throughout the code,
410 that you should use in your code. Do not use these variable names for
411 anything other than how they are normally used, and do not use any
412 other variable names in their place. Some of these are:
414 $self # first named argument in object method
416 Subroutines (except for special cases, like AUTOLOAD and simple accessors)
417 begin with a verb, with words following to complete the action. Accessors
418 don't start with "Get" if they're just the name of the attribute.
420 Accessors which return an object should end with the suffix Obj.
422 This section needs clarification for RT.
424 Words begin with a capital letter. They
425 should as clearly as possible describe the activity to be peformed, and
426 the data to be returned.
434 Subroutines beginning with C<_> are special: they are not to be used
435 outside the current object. There is not to be enforced by the code
436 itself, but by someone very big and very scary.
438 For large for() loops, do not use $_, but name the variable.
439 Do not use $_ (or assume it) except for when it is absolutely
440 clear what is going on, or when it is required (such as with
444 print; # OK; everyone knows this one
445 print uc; # wrong; few people know this
446 print uc $_; # better
449 Note that the special variable C<_> I<should> be used when possible.
450 It is a placeholder that can be passed to stat() and the file test
451 operators, that saves perl a trip to re-stat the file. In the
452 example below, using C<$file> over for each file test, instead of
453 C<_> for subsequent uses, is a performance hit. You should be
454 careful that the last-tested file is what you think it is, though.
456 if (-d $file) { # $file is a directory
458 } elsif (-l _) { # $file is a symlink
462 Package names begin with a capital letter in each word, followed by
463 lower case letters (for the most part). Multiple words should be StudlyCapped.
466 RT::Database::MySQL # proper name
467 RT::Display::Provider # good
468 RT::CustomField # not so good, but OK
470 Plugin modules should begin with "RTx::", followed by the name
473 =head1 Code formatting
475 Use perltidy. Anything we say here is wrong if it conflicts with what
476 perltidy does. Your perltidyrc should read:
478 -lp -vt=2 -vtc=2 -nsfs -bar
480 =head2 Indents and Blank Space
482 All indents should be tabs. Set your tab stops whatever you want them
483 to be; I use 8 spaces per tabs.
485 No space before a semicolon that closes a statement.
490 Line up corresponding items vertically.
496 open(FILE, $fh) or die $!;
497 open(FILE2, $fh2) or die $!;
499 $rot13 =~ tr[abcedfghijklmnopqrstuvwxyz]
500 [nopqrstuvwxyzabcdefghijklm];
502 # note we use a-mn-z instead of a-z,
507 Put blank lines between groups of code that do different things. Put
508 blank lines after your variable declarations. Put a blank line before a
509 final return() statement. Put a blank line following a block (and
510 before, with the exception of comment lines).
514 # this is my function!
517 my $obj = new Constructor;
532 For control structures, there is a space between the keyword and opening
533 parenthesis. For functions, there is not.
541 Be careful about list vs. scalar context with parentheses!
543 my @array = ('a', 'b', 'c');
544 my($first_element) = @array; # a
545 my($first_element) = ('a', 'b', 'c'); # a
546 my $element_count = @array; # 3
547 my $last_element = ('a', 'b', 'c'); # c
549 Always include parentheses after functions, even if there are no arguments.
550 There are some exceptions, such as list operators (like print) and unary
551 operators (like undef, delete, uc).
553 There is no space inside the parentheses, unless it is needed for
556 for ( map { [ $_, 1 ] } @list ) # OK
557 for ( @list ) # not really OK, not horrible
559 On multi-line expressions, match up the closing parenthesis with either
560 the opening statement, or the opening parenthesis, whichever works best.
568 if ($foo && $bar && $baz
574 Whether or not there is space following a closing parenthesis is
575 dependent on what it is that follows.
577 print foo(@bar), baz(@buz) if $xyzzy;
579 Note also that parentheses around single-statement control expressions,
580 as in C<if $xyzzy>, are optional (and discouraged) C<if> it is I<absolutely>
581 clear -- to a programmer -- what is going on. There is absolutely no
582 need for parentheses around C<$xyzzy> above, so leaving them out enhances
583 readability. Use your best discretion. Better to include them, if
584 there is any question.
586 The same essentially goes for perl's built-in functions, when there is
587 nothing confusing about what is going on (for example, there is only one
588 function call in the statement, or the function call is separated by a
589 flow control operator). User-supplied functions must always include
592 print 1, 2, 3; # good
593 delete $hash{key} if isAnon($uid); # good
596 However, if there is any possible confusion at all, then include the
597 parentheses. Remember the words of Larry Wall in the perlstyle manpage:
599 When in doubt, parenthesize. At the very least it will
600 let some poor schmuck bounce on the % key in vi.
602 Even if you aren't in doubt, consider the mental welfare
603 of the person who has to maintain the code after you, and
604 who will probably put parens in the wrong place.
606 So leave them out when it is absoutely clear to a programmer, but if
607 there is any question, leave them in.
612 (This is about control braces, not hash/data structure braces.)
614 There is always a space befor the opening brace.
616 while (<$fh>){ # wrong
617 while (<$fh>) { # right
619 A one-line block may be put on one line, and the semicolon may be
622 for (@list) { print }
624 Otherwise, finish each statement with a semicolon, put the keyword and
625 opening curly on the first line, and the ending curly lined up with the
633 Generally, we prefer "uncuddled elses":
642 _If_ the if statement is very brief, sometimes "cuddling" the else makes code more readable. Feel free to cuddle them in that case:
653 Put space around most operators. The primary exception is the for
654 aesthetics; e.g., sometimes the space around "**" is ommitted,
655 and there is never a space before a ",", but always after.
657 print $x , $y; # wrong
658 print $x, $y; # right
663 Note that "&&" and "||" have a higher precedence than "and" and "or".
664 Other than that, they are exactly the same. It is best to use the lower
665 precedence version for control, and the higher for testing/returning
668 $bool = $flag1 or $flag2; # WRONG (doesn't work)
669 $value = $foo || $bar; # right
670 open(FILE, $file) or die $!;
672 $true = foo($bar) && baz($buz);
673 foo($bar) and baz($buz);
675 Note that "and" is seldom ever used, because the statement above is
676 better written using "if":
678 baz($buz) if foo($bar);
680 Most of the time, the confusion between and/&&, or/|| can be alleviated
681 by using parentheses. If you want to leave off the parentheses then you
682 I<must> use the proper operator. But if you use parentheses -- and
683 normally, you should, if there is any question at all -- then it doesn't
684 matter which you use. Use whichever is most readable and aesthetically
685 pleasing to you at the time, and be consistent within your block of code.
687 Break long lines AFTER operators, except for "and", "or", "&&", "||".
688 Try to keep the two parts to a binary operator (an operator that
689 has two operands) together when possible.
691 print "foo" . "bar" . "baz"
694 print "foo" . "bar" . "baz" .
697 print $foo unless $x == 3 && $y ==
698 4 && $z == 5; # wrong
700 print $foo unless $x == 3 && $y == 4
706 Put space around a complex subscript inside the brackets or braces.
708 $foo{$bar{baz}{buz}}; # OK
709 $foo{ $bar{baz}{buz} }; # better
711 In general, use single-quotes around literals, and double-quotes
712 when the text needs to be interpolated.
714 It is OK to omit quotes around names in braces and when using
715 the => operator, but be careful not to use a name that doubles as
716 a function; in that case, quote.
718 $what{'time'}{it}{is} = time();
720 When making compound statements, put the primary action first.
722 open(FILE, $fh) or die $!; # right
723 die $! unless open(FILE, $fh); # wrong
725 print "Starting\n" if $verbose; # right
726 $verbose && print "Starting\n"; # wrong
729 Use here-docs instead of repeated print statements.
732 This is a whole bunch of text.
733 I like it. I don't need to worry about messing
734 with lots of print statements and lining them up.
737 Just remember that unless you put single quotes around your here-doc
738 token (<<'EOT'), the text will be interpolated, so escape any "$" or "@"
741 =head1 INTERNATIONALIZATION
744 =head2 String extraction styleguide
750 Templates should use the /l filtering component to call the localisation
755 Should become <&|/l&>Foo!</&>
757 All newlines should be removed from localized strings, to make it easy to
758 grep the codebase for strings to be localized
764 Should become <&|/l&>Foo Bar Baz</&>
767 Variable subsititutions should be moved to Locale::MakeText format
769 The string Hello, <%$name %>
771 should become <&|/l, $name &>Hello, [_1]</&>
774 Multiple variables work just like single variables
776 The string You found <%$num%> tickets in queue <%$queue%>
778 should become <&|/l, $num, $queue &>You found [_1] tickets in queue [_2]</&>
780 When subcomponents are called in the middle of a phrase, they need to be escaped
783 The string <input type="submit" value="New ticket in"> <& /Elements/SelectNewTicketQueue&>
785 should become <&|/l, $m->scomp('/Elements/SelectNewTicketQueue')&><input type="submit" value="New ticket in"> [_1]</&>
790 The string <& /Elements/TitleBoxStart, width=> "40%", titleright => "RT $RT::VERSION for $RT::rtname", title => 'Login' &>
792 should become <& /Elements/TitleBoxStart,
794 titleright => loc("RT [_1] for [_2]",$RT::VERSION, $RT::rtname),
795 title => loc('Login'),
803 Within RT's core code, every module has a localization handle available through the 'loc' method:
805 The code return ( $id, "Queue created" );
807 should become return ( $id, $self->loc("Queue created") );
809 When returning or localizing a single string, the "extra" set of parenthesis () should be omitted.
811 The code return ("Subject changed to ". $self->Data );
813 should become return $self->loc( "Subject changed to [_1]", $self->Data );
816 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.
821 =head1 CODING PRCEDURE
823 This is for new programs, modules, specific APIs, or anything else.
827 =item Present idea to rt-devel
829 We may know of a better way to approach the problem, or know of an
830 existing way to deal with it, or know someone else is working on it.
831 This is mostly informal, but a fairly complete explanation for the need
832 and use of the code should be provided.
835 =item Present complete specs to rt-devel
837 The complete proposed API should be submitted for
838 approval and discussion. For web and command-line programs, present the
839 functionality and interface (op codes, command-lin switches, etc.).
841 The best way to do this is to take the documentation portion of the
842 boilerplate and fill it in. You can make changes later if necessary,
843 but fill it in as much as you can.
847 =item Prepare for code review
849 When you are done, the code will undergo a code review by a member of
850 the core team, or someone picked by the core team. This is not to
851 belittle you (that's just a nice side effect), it is to make sure that
852 you understand your code, that we understand your code, that it won't
853 break other code, that it follows the documentation and existing
854 proposal. It is to check for possible optimizations or better ways of
857 Note that all code is expected to follow the coding principles and style
858 guide contained in this document.
863 After the code is done (possibly going through multiple code reviews),
864 if you do not have repository access, submit it to rt-<major-version>-bugs@fsck.com as a unified diff. From that point on, it'll be handled by someone with repository access.
869 =head1 BUG REPORTS, PATCHES
871 Use rt-<major-version>-bugs@fsck.com for I<any> bug that is not
872 being fixed immediately. If it is not in RT, there
873 is a good chance it will not be dealt with.
875 Send patches to rt-<major-version>-bugs@fsck.com, too. Use C<diff
880 RT uses a convention to denote the foreign key status in its tables.
881 The rule of thumb is:
885 =item When it references to another table, always use the table name
887 For example, the C<Template> field in the C<Scrips> table refers to
888 the C<Id> of the same-named C<Template> table.
890 =item Otherwise, always use the C<Id> suffix
892 For example, the C<ObjectId> field in the C<ACL> table can refer
893 to any object, so it has the C<Id> suffix.
897 There are some legacy fields that did not follow this rule, namely
898 C<ACL.PrincipalId>, C<GroupMembers.GroupId> and C<Attachments.TransactionId>,
899 but new tables are expected to be consistent.
903 Talk about DBIx::SearchBuilder
907 cascading style sheets
909 Talk about adding a new translation
911 Talk more about logging
915 Adapted from Slash Styleguide by jesse - 20 Dec, 2002