Merge branch 'master' of https://github.com/jgoodman/Freeside
[freeside.git] / rt / lib / RT / StyleGuide.pod
index 95b2e3a..d958c87 100644 (file)
@@ -40,10 +40,8 @@ We hope to add any significant changes at the bottom of the document.
 
 =head2 Perl Version
 
-We code everything to perl 5.6.1. Some features require advanced unicode
-features in perl 5.8.0. It is acceptable that unicode features work only for 
-US-ASCII on perl 5.6.1. 
-
+We code everything to perl 5.8.3 or higher.  Complete unicode support
+requires bugfixes found in 5.8.3.
 
 =head2 Documentation
 
@@ -239,25 +237,39 @@ leads to cleaner code.
         my $var1 = shift;                       # right
         my $var2 = shift;                       
 
+=head2 Method parameters
 
+If a method takes exactly one mandatory argument, the argument should be
+passed in a straightforward manner:
 
-=head2 Tests
+        my $self = shift;
+        my $id = shift;
 
-Modules should provide test code, with documentation on how to use
-it. Test::Inline allows tests to be embedded in code. Test::More makes it 
-easy to create tests. Any code you write should have a testsuite.
-Any code you alter should have a test suite. If a patch comes in without
-tests, there is something wrong.
+In all other cases, the method needs to take named parameters, usually
+using a C<%args> hash to store them:
+
+        my $self = shift;
+        my %args = ( Name => undef,
+                     Description => undef,
+                     @_ );
 
-When altering code, you must run the test harness before submitting a patch
-or committing code to the repository. 
+You may specify defaults to those named parameters instead of using
+C<undef> above, as long as it is documented as such.
 
-"make regression" will extract inline tests, blow away the system database
-and run the test suite.
+It is worth noting that the existing RT codebase had not followed this
+style perfectly; we are trying to fix it without breaking exsiting APIs.
 
-"make regression-quiet" will do all that and not print the "ok" lines.
+=head2 Tests
+
+Modules should provide test code, with documentation on how to use
+it.  Test::More makes it easy to create tests. Any code you write
+should have a testsuite.  Any code you alter should have a test
+suite. If a patch comes in without tests, there is something wrong.
 
+When altering code, you must run the test harness before submitting a
+patch or committing code to the repository.
 
+"make test" will run the test suite.
 
 =head2 STDIN/STDOUT
 
@@ -769,11 +781,11 @@ should become             <&|/l, $m->scomp('/Elements/SelectNewTicketQueue')&><input type="
 
 
 
-The string     <& /Elements/TitleBoxStart, width=> "40%", titleright => "RT $RT::VERSION for   $RT::rtname", title => 'Login' &>
+The string     <& /Elements/TitleBoxStart, width=> "40%", titleright => "RT $RT::VERSION for   RT->Config->Get('rtname')", title => 'Login' &>
 
 should become  <& /Elements/TitleBoxStart, 
                        width=> "40%",
-                       titleright => loc("RT [_1] for [_2]",$RT::VERSION, $RT::rtname),
+                       titleright => loc("RT [_1] for [_2]",$RT::VERSION, RT->Config->Get('rtname')),
                        title => loc('Login'),
                &>
        
@@ -798,17 +810,15 @@ should become      return $self->loc( "Subject changed to [_1]", $self->Data );
 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.
 
 
-=back 4
+=back
 
 =head1 CODING PRCEDURE
 
 This is for new programs, modules, specific APIs, or anything else.
 
-Contact for core team is the slashcode-development mailing list.
-
 =over 4
 
-=item Present idea to core team
+=item Present idea to rt-devel
 
 We may know of a better way to approach the problem, or know of an
 existing way to deal with it, or know someone else is working on it. 
@@ -816,9 +826,9 @@ This is mostly informal, but a fairly complete explanation for the need
 and use of the code should be provided.
 
 
-=item Present complete specs to core team
+=item Present complete specs to rt-devel
 
-The complete proposed API to the core team should be submitted for
+The complete proposed API  should be submitted for
 approval and discussion.  For web and command-line programs, present the
 functionality and interface (op codes, command-lin switches, etc.).
 
@@ -827,13 +837,8 @@ boilerplate and fill it in.  You can make changes later if necessary,
 but fill it in as much as you can.
 
 
-=item Announce any changes to interface
-
-If the way it works or how it is called is going to change, notify the core
-team.
 
-
-=item Prepare for core review
+=item Prepare for code review
 
 When you are done, the code will undergo a code review by a member of
 the core team, or someone picked by the core team.  This is not to
@@ -843,9 +848,6 @@ break other code, that it follows the documentation and existing
 proposal.  It is to check for possible optimizations or better ways of
 doing it.
 
-For members of the core team, one or more other members of the team will
-perform the review.
-
 Note that all code is expected to follow the coding principles and style
 guide contained in this document.
 
@@ -867,7 +869,44 @@ is a good chance it will not be dealt with.
 Send patches to rt-<major-version>-bugs@fsck.com, too.  Use C<diff
 -u> for patches.
 
+=head1 SCHEMA DESIGN
+
+RT uses a convention to denote the foreign key status in its tables.
+The rule of thumb is:
+
+=over 4
+
+=item When it references to another table, always use the table name
+
+For example, the C<Template> field in the C<Scrips> table refers to
+the C<Id> of the same-named C<Template> table.
+
+=item Otherwise, always use the C<Id> suffix
+
+For example, the C<ObjectId> field in the C<ACL> table can refer
+to any object, so it has the C<Id> suffix.
+
+=back
+
+There are some legacy fields that did not follow this rule, namely
+C<ACL.PrincipalId>, C<GroupMembers.GroupId> and C<Attachments.TransactionId>,
+but new tables are expected to be consistent.
+
+
+=head1 EXTENDING RT CLASSES
+
+=head2 The Overlay mechanism
+
+RT's classes allow "overlay" methods to be placed into files named Filename_Vendor.pm and Filename_Local.pm
+_Vendor is for 3rd-party vendor add-ons, while _Local is for site-local customizations.
+
+These overlay files can contain new subs or subs to replace existing subs in this module.
+
+Each of these files should begin with the line
+
+   no warnings qw(redefine);
 
+so that perl does not kick and scream when you redefine a subroutine or variable in your overlay.
 
 =head1 TO DO