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 first start a discussion in the RT Developers category on the community
24 forum at L<https://forum.bestpractical.com>.
26 Note that with much of this document, it is not so much the Right Way as
27 it is Our Way. We need to have conventions in order to make life easier
28 for everyone. So don't gripe, and just follow it, because you didn't
29 get a good grade in "Plays Well With Others" in kindergarten and you
30 want to make up for it now.
32 We don't always follow this guide. We are making changes throughout
33 our code to be in line with it. But just because we didn't do
34 it yet, that is no excuse. Do it anyway. :-)
36 This document is subject to change at the whims of the core RT team.
37 We hope to add any significant changes at the bottom of the document.
40 =head1 CODING PRINCIPLES
44 We code everything to Perl 5.10.1 or higher.
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 may end in "rc" and a number if they are release candidates:
97 2.0.0rc1 First release candiate for real 2.0.0
102 All code should be self-documenting as much as possible. Only include
103 necessary comments. Use names like "$ticket_count", so you don't need to
109 Include any comments that are, or might be, necessary in order for
110 someone else to understand the code. Sometimes a simple one-line
111 comment is good to explain what the purpose of the following code is
112 for. Sometimes each line needs to be commented because of a complex
113 algorithm. Read Kernighan & Pike's I<Practice of Programming> about
114 commenting. Good stuff, Maynard.
117 =head2 Warnings and Strict
119 All code must compile and run cleanly with "use strict" enabled and the
120 perl "-w" (warnings) option on. If you must do something that -w or
121 strict complains about, there are workarounds, but the chances that you
122 really need to do it that way are remote.
124 =head2 Lexical Variables
126 Use only lexical variables, except for special global variables
127 ($VERSION, %ENV, @ISA, $!, etc.) or very special circumstances (see
128 %HTML::Mason::Commands::session ). Global variables
129 for regular use are never appropriate. When necessary, "declare"
130 globals with "use vars" or "our()".
132 A lexical variable is created with my(). A global variable is
133 pre-existing (if it is a special variable), or it pops into existence
134 when it is used. local() is used to tell perl to assign a temporary
135 value to a variable. This should only be used with special variables,
136 like $/, or in special circumstances. If you must assign to any global
137 variable, consider whether or not you should use local().
139 local() may also be used on elements of arrays and hashes, though there
140 is seldom a need to do it, and you shouldn't.
143 =head2 Pass by Reference
145 Arrays and hashes should be passed to and from functions by reference
146 only. Note that a list and an array are NOT the same thing. This
149 return($user, $form, $constants);
151 An exception might be a temporary array of discrete arguments:
153 my @return = ($user, $form);
154 push @return, $constants if $flag;
157 Although, usually, this is better (faster, easier to read, etc.):
160 return($user, $form, $constants);
162 return($user, $form);
165 We need to talk about Class::ReturnValue here.
168 =head2 Method parameters
170 If a method takes exactly one mandatory argument, the argument should be
171 passed in a straightforward manner:
176 In all other cases, the method needs to take named parameters, usually
177 using a C<%args> hash to store them:
182 Description => undef,
186 You may specify defaults to those named parameters instead of using
187 C<undef> above, as long as it is documented as such.
189 It is worth noting that the existing RT codebase had not followed this
190 style perfectly; we are trying to fix it without breaking existing APIs.
194 Modules should provide test code, with documentation on how to use
195 it. Test::More makes it easy to create tests. Any code you write
196 should have a testsuite. Any code you alter should have a test
197 suite. If a patch comes in without tests, there is something wrong.
199 When altering code, you must run the test harness before submitting a
200 patch or committing code to the repository.
202 "make test" will run the test suite.
206 Always report errors using $RT::Logger. It's a Log::Dispatch object.
207 Unlike message meant for the user, log messages are not to be
210 There are several different levels ($RT::Logger methods) of logging:
216 Used for messages only needed during system debugging.
220 Should be used to describe "system-critical" events which aren't errors.
221 Examples: creating users, deleting users, creating tickets, creating queues,
222 sending email (message id, time, recipients), recieving mail, changing
223 passwords, changing access control, superuser logins)
227 Used for RT-generated failures during execution.
231 Should be used for messages when an action can not be completed due to some
232 error condition beyond our control.
236 In the web UI and modules, never print directly to STDERR. Do not print
237 directly to STDOUT, unless you need to print directly to the user's console.
239 In command-line programs, feel free to print to STDERR and STDOUT as
240 needed for direct console communication. But for actual error reporting,
246 Always check return values from system calls, including open(),
247 close(), mkdir(), or anything else that talks directly to the system.
248 Perl built-in system calls return the error in $!; some functions in
249 modules might return an error in $@ or some other way, so read the module's
250 documentation if you don't know. Always do something, even if it is
251 just calling $RT::Logger->warning(), when the return value is not what you'd expect.
257 Much of the style section is taken from the perlsyle manpage. We make
258 some changes to it here, but it wouldn't be a bad idea to read that
265 =item function vs. sub(routine) vs. method
267 Just because it is the Perl Way (not necessarily right for all
268 languages, but the documented terminology in the perl documentation),
269 "method" should be used only to refer to a subroutine that are object
270 methods or class methods; that is, these are functions that are used
271 with OOP that always take either an object or a class as the first
272 argument. Regular subroutines, ones that are not object or class
273 methods, are functions. Class methods that create and return an object
274 are optionally called constructors.
278 "users" are normally users of RT, the ones hitting the site; if using
279 it in any other context, specify.
280 "system users" are user
281 names on the operating system. "database users" are the user names in
282 the database server. None of these needs to be capitalized.
289 Don't use single-character variables, except as iterator variables.
291 Don't use two-character variables just to spite us over the above rule.
293 Constants are in all caps; these are variables whose value will I<never>
294 change during the course of the program.
296 $Minimum = 10; # wrong
297 $MAXIMUM = 50; # right
299 Other variables are lowercase, with underscores separating the words.
300 They words used should, in general, form a noun (usually singular),
301 unless the variable is a flag used to denote some action that should be
302 taken, in which case they should be verbs (or gerunds, as appropriate)
303 describing that action.
305 $thisVar = 'foo'; # wrong
306 $this_var = 'foo'; # right
307 $work_hard = 1; # right, verb, boolean flag
308 $running_fast = 0; # right, gerund, boolean flag
310 Arrays and hashes should be plural nouns, whether as regular arrays and
311 hashes or array and hash references. Do not name references with "ref"
312 or the data type in the name.
314 @stories = (1, 2, 3); # right
315 $comment_ref = [4, 5, 6]; # wrong
316 $comments = [4, 5, 6]; # right
317 $comment = $comments->[0]; # right
319 Make the name descriptive. Don't use variables like "$sc" when you
320 could call it "$story_count". See L<"Comments">.
322 There are several variables in RT that are used throughout the code,
323 that you should use in your code. Do not use these variable names for
324 anything other than how they are normally used, and do not use any
325 other variable names in their place. Some of these are:
327 $self # first named argument in object method
329 Subroutines (except for special cases, like AUTOLOAD and simple accessors)
330 begin with a verb, with words following to complete the action. Accessors
331 don't start with "Get" if they're just the name of the attribute.
333 Accessors which return an object should end with the suffix Obj.
335 This section needs clarification for RT.
337 Words begin with a capital letter. They
338 should as clearly as possible describe the activity to be peformed, and
339 the data to be returned.
347 Subroutines beginning with C<_> are special: they are not to be used
348 outside the current object. There is not to be enforced by the code
349 itself, but by someone very big and very scary.
351 For large for() loops, do not use $_, but name the variable.
352 Do not use $_ (or assume it) except for when it is absolutely
353 clear what is going on, or when it is required (such as with
357 print; # OK; everyone knows this one
358 print uc; # wrong; few people know this
359 print uc $_; # better
362 Note that the special variable C<_> I<should> be used when possible.
363 It is a placeholder that can be passed to stat() and the file test
364 operators, that saves perl a trip to re-stat the file. In the
365 example below, using C<$file> over for each file test, instead of
366 C<_> for subsequent uses, is a performance hit. You should be
367 careful that the last-tested file is what you think it is, though.
369 if (-d $file) { # $file is a directory
371 } elsif (-l _) { # $file is a symlink
375 Package names begin with a capital letter in each word, followed by
376 lower case letters (for the most part). Multiple words should be StudlyCapped.
379 RT::Database::MySQL # proper name
380 RT::Display::Provider # good
381 RT::CustomField # not so good, but OK
383 Plugin modules should begin with "RT::Extension::", followed by the name
386 =head1 Code formatting
388 When in doubt, use perltidy; RT includes a F<.perltidyrc>.
390 =head2 Indents and Blank Space
392 All indents should be four spaces; hard tabs are forbidden.
394 No space before a semicolon that closes a statement.
399 Line up corresponding items vertically.
405 open(FILE, $fh) or die $!;
406 open(FILE2, $fh2) or die $!;
408 $rot13 =~ tr[abcedfghijklmnopqrstuvwxyz]
409 [nopqrstuvwxyzabcdefghijklm];
411 # note we use a-mn-z instead of a-z,
416 Put blank lines between groups of code that do different things. Put
417 blank lines after your variable declarations. Put a blank line before a
418 final return() statement. Put a blank line following a block (and
419 before, with the exception of comment lines).
423 # this is my function!
426 my $obj = new Constructor;
440 For control structures, there is a space between the keyword and opening
441 parenthesis. For functions, there is not.
449 Be careful about list vs. scalar context with parentheses!
451 my @array = ('a', 'b', 'c');
452 my($first_element) = @array; # a
453 my($first_element) = ('a', 'b', 'c'); # a
454 my $element_count = @array; # 3
455 my $last_element = ('a', 'b', 'c'); # c
457 Always include parentheses after functions, even if there are no arguments.
458 There are some exceptions, such as list operators (like print) and unary
459 operators (like undef, delete, uc).
461 There is no space inside the parentheses, unless it is needed for
464 for ( map { [ $_, 1 ] } @list ) # OK
465 for ( @list ) # not really OK, not horrible
467 On multi-line expressions, match up the closing parenthesis with either
468 the opening statement, or the opening parenthesis, whichever works best.
476 if ($foo && $bar && $baz
481 Whether or not there is space following a closing parenthesis is
482 dependent on what it is that follows.
484 print foo(@bar), baz(@buz) if $xyzzy;
486 Note also that parentheses around single-statement control expressions,
487 as in C<if $xyzzy>, are optional (and discouraged) C<if> it is I<absolutely>
488 clear -- to a programmer -- what is going on. There is absolutely no
489 need for parentheses around C<$xyzzy> above, so leaving them out enhances
490 readability. Use your best discretion. Better to include them, if
491 there is any question.
493 The same essentially goes for perl's built-in functions, when there is
494 nothing confusing about what is going on (for example, there is only one
495 function call in the statement, or the function call is separated by a
496 flow control operator). User-supplied functions must always include
499 print 1, 2, 3; # good
500 delete $hash{key} if isAnon($uid); # good
503 However, if there is any possible confusion at all, then include the
504 parentheses. Remember the words of Larry Wall in the perlstyle manpage:
506 When in doubt, parenthesize. At the very least it will
507 let some poor schmuck bounce on the % key in vi.
509 Even if you aren't in doubt, consider the mental welfare
510 of the person who has to maintain the code after you, and
511 who will probably put parens in the wrong place.
513 So leave them out when it is absoutely clear to a programmer, but if
514 there is any question, leave them in.
519 (This is about control braces, not hash/data structure braces.)
521 There is always a space befor the opening brace.
523 while (<$fh>){ # wrong
524 while (<$fh>) { # right
526 A one-line block may be put on one line, and the semicolon may be
529 for (@list) { print }
531 Otherwise, finish each statement with a semicolon, put the keyword and
532 opening curly on the first line, and the ending curly lined up with the
540 Generally, we prefer "cuddled elses":
550 Put space around most operators. The primary exception is the for
551 aesthetics; e.g., sometimes the space around "**" is ommitted,
552 and there is never a space before a ",", but always after.
554 print $x , $y; # wrong
555 print $x, $y; # right
560 Note that "&&" and "||" have a higher precedence than "and" and "or".
561 Other than that, they are exactly the same. It is best to use the lower
562 precedence version for control, and the higher for testing/returning
565 $bool = $flag1 or $flag2; # WRONG (doesn't work)
566 $value = $foo || $bar; # right
567 open(FILE, $file) or die $!;
569 $true = foo($bar) && baz($buz);
570 foo($bar) and baz($buz);
572 Note that "and" is seldom ever used, because the statement above is
573 better written using "if":
575 baz($buz) if foo($bar);
577 Most of the time, the confusion between and/&&, or/|| can be alleviated
578 by using parentheses. If you want to leave off the parentheses then you
579 I<must> use the proper operator. But if you use parentheses -- and
580 normally, you should, if there is any question at all -- then it doesn't
581 matter which you use. Use whichever is most readable and aesthetically
582 pleasing to you at the time, and be consistent within your block of code.
584 Break long lines AFTER operators, except for ".", "and", "or", "&&", "||".
585 Try to keep the two parts to a binary operator (an operator that
586 has two operands) together when possible.
588 print "foo" . "bar" . "baz" .
591 print "foo" . "bar" . "baz"
594 print $foo unless $x == 3 && $y ==
595 4 && $z == 5; # wrong
597 print $foo unless $x == 3 && $y == 4
603 Put space around a complex subscript inside the brackets or braces.
605 $foo{$bar{baz}{buz}}; # OK
606 $foo{ $bar{baz}{buz} }; # better
608 In general, use single-quotes around literals, and double-quotes
609 when the text needs to be interpolated.
611 It is OK to omit quotes around names in braces and when using
612 the => operator, but be careful not to use a name that doubles as
613 a function; in that case, quote.
615 $what{'time'}{it}{is} = time();
617 When making compound statements, put the primary action first.
619 open(FILE, $fh) or die $!; # right
620 die $! unless open(FILE, $fh); # wrong
622 print "Starting\n" if $verbose; # right
623 $verbose && print "Starting\n"; # wrong
626 Use here-docs instead of repeated print statements.
629 This is a whole bunch of text.
630 I like it. I don't need to worry about messing
631 with lots of print statements and lining them up.
634 Just remember that unless you put single quotes around your here-doc
635 token (<<'EOT'), the text will be interpolated, so escape any "$" or "@"
638 =head1 INTERNATIONALIZATION
641 =head2 String extraction styleguide
647 Templates should use the /l filtering component to call the localisation
652 Should become <&|/l&>Foo!</&>
654 All newlines should be removed from localized strings, to make it easy to
655 grep the codebase for strings to be localized
661 Should become <&|/l&>Foo Bar Baz</&>
664 Variable subsititutions should be moved to Locale::MakeText format
666 The string Hello, <%$name %>
668 should become <&|/l, $name &>Hello, [_1]</&>
671 Multiple variables work just like single variables
673 The string You found <%$num%> tickets in queue <%$queue%>
675 should become <&|/l, $num, $queue &>You found [_1] tickets in queue [_2]</&>
677 When subcomponents are called in the middle of a phrase, they need to be escaped
680 The string <input type="submit" value="New ticket in"> <& /Elements/SelectNewTicketQueue&>
682 should become <&|/l, $m->scomp('/Elements/SelectNewTicketQueue')&><input type="submit" value="New ticket in"> [_1]</&>
687 The string <& /Widgets/TitleBoxStart, width=> "40%", titleright => "RT $RT::VERSION for RT->Config->Get('rtname')", title => 'Login' &>
689 should become <& /Widgets/TitleBoxStart,
691 titleright => loc("RT [_1] for [_2]",$RT::VERSION, RT->Config->Get('rtname')),
692 title => loc('Login'),
699 Within RT's core code, every module has a localization handle available through the 'loc' method:
701 The code return ( $id, "Queue created" );
703 should become return ( $id, $self->loc("Queue created") );
705 When returning or localizing a single string, the "extra" set of parenthesis () should be omitted.
707 The code return ("Subject changed to ". $self->Data );
709 should become return $self->loc( "Subject changed to [_1]", $self->Data );
712 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.
717 =head1 CODING PRCEDURE
719 This is for new programs, modules, specific APIs, or anything else.
723 =item Create a topic in RT Developers on the Forum
725 We may know of a better way to approach the problem, or know of an
726 existing way to deal with it, or know someone else is working on it.
727 This is mostly informal, but a fairly complete explanation for the need
728 and use of the code should be provided.
731 =item Present specs in RT Developers
733 The complete proposed API should be submitted for
734 discussion. For web and command-line programs, present the
735 functionality and interface (op codes, command-line switches, etc.).
737 The best way to do this is to take the documentation portion of the
738 boilerplate and fill it in. You can make changes later if necessary,
739 but fill it in as much as you can.
743 =item Prepare for code review
745 When you are done, the code will undergo a code review by a member of
746 the core team, or someone picked by the core team. This is not to
747 belittle you (that's just a nice side effect), it is to make sure that
748 you understand your code, that we understand your code, that it won't
749 break other code, that it follows the documentation and existing
750 proposal. It is to check for possible optimizations or better ways of
753 Note that all code is expected to follow the coding principles and style
754 guide contained in this document.
759 After the code is done (possibly going through multiple code reviews),
760 submit your updates as a pull request on GitHub. If you don't have a GitHub
761 account, you can generate patches and send email to rt-bugs@bestpractical.com
762 which will create a ticket in our public issue tracker at
763 L<https://issues.bestpractical.com>.
768 =head1 BUG REPORTS, PATCHES
770 Use rt-bugs@bestpractical.com for I<any> bug that is not being fixed
771 immediately. If it is not in RT, there is a good chance it will not be
774 Send patches to rt-bugs@bestpractical.com, too. Use C<diff -u> for
779 RT uses a convention to denote the foreign key status in its tables.
780 The rule of thumb is:
784 =item When it references to another table, always use the table name
786 For example, the C<Template> field in the C<Scrips> table refers to
787 the C<Id> of the same-named C<Template> table.
789 =item Otherwise, always use the C<Id> suffix
791 For example, the C<ObjectId> field in the C<ACL> table can refer
792 to any object, so it has the C<Id> suffix.
796 There are some legacy fields that did not follow this rule, namely
797 C<ACL.PrincipalId>, C<GroupMembers.GroupId> and C<Attachments.TransactionId>,
798 but new tables are expected to be consistent.
801 =head1 EXTENDING RT CLASSES
803 =head2 The Overlay mechanism
805 RT's classes allow "overlay" methods to be placed into files named Filename_Vendor.pm and Filename_Local.pm
806 _Vendor is for 3rd-party vendor add-ons, while _Local is for site-local customizations.
808 These overlay files can contain new subs or subs to replace existing subs in this module.
810 Each of these files should begin with the line
812 no warnings qw(redefine);
814 so that perl does not kick and scream when you redefine a subroutine or variable in your overlay.
818 Talk about DBIx::SearchBuilder
822 cascading style sheets
824 Talk about adding a new translation
826 Talk more about logging