[Rt-commit] rt branch, 4.2/contributing, created. rt-4.2.10-225-gbc28fe9

Alex Vandiver alexmv at bestpractical.com
Tue May 5 00:13:42 EDT 2015


The branch, 4.2/contributing has been created
        at  bc28fe98fcde5a7446fe29c23798177c48e27b89 (commit)

- Log -----------------------------------------------------------------
commit cbb66f8d70310036fdbb743f35c2538af455c0f6
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue May 5 00:03:55 2015 -0400

    Remove out-of-date RT::StyleGuide, and replace with "use perltidy"

diff --git a/docs/hacking.pod b/docs/hacking.pod
index 23ce51e..6ba056e 100644
--- a/docs/hacking.pod
+++ b/docs/hacking.pod
@@ -46,27 +46,20 @@ Branches should be reviewed by another developer before being merged.
 Reviewers should make sure that the branch accomplishes what it claims
 to, and does not introduce any unwanted behavior in doing so.  Commit
 messages explain the B<why> as much as the B<what> of each commit, and
-not include extranous changes.
+not include extraneous changes.
 
 
 =head1 Code conventions
 
-The RT codebase is more than ten years old; as such, there are sections
-which do not (yet) conform to the guidelines below.  Please attempt to
-follow the guidelines, even if the code surrounding your changes does
-not yet.
+The most salient convention is that RT uses four spaces for indentation,
+and eschews tabs entirely; a F<.perltidyrc> is included for use with
+L<perltidy|https://metacpan.org/pod/distribution/Perl-Tidy/bin/perltidy>.
 
-RT also includes a F<.perltidyrc> in its top-level which encodes many of
-the conventions.
-
-=over
-
-=item Indentation
-
-Each level of indentation should be four spaces; tabs should never be
-used for indentation.
-
-=back
+Note that the RT codebase is more than ten years old; as such, there are
+sections which do not (yet) conform to the perltidyrc.  Please attempt
+to follow its guidelines, even if the code surrounding your changes does
+not yet.  Better yet, include a commit which fixes the style of the
+surrounding code first.
 
 =head1 Internationalization
 
diff --git a/lib/RT/StyleGuide.pod b/lib/RT/StyleGuide.pod
deleted file mode 100644
index 3a75562..0000000
--- a/lib/RT/StyleGuide.pod
+++ /dev/null
@@ -1,828 +0,0 @@
-=head1 NAME
-
-RT::StyleGuide - RT Style Guide
-
-=head1 CAVEATS
-
-This file is somewhat out of date; L<hacking> takes precedence over it.
-
-=head1 INTRODUCTION
-
-All code and documentation that is submitted to be included in the RT
-distribution should follow the style in this document.  This is not to
-try to stifle your creativity, but to make life easier for everybody who
-has to work with your code, and to aid those who are not quite sure how
-to do something.
-
-These conventions below apply to perl modules, web programs, and
-command-line programs, specifically, but also might apply to some
-degree to any Perl code written for use in RT.
-
-Note that these are all guidelines, not unbreakable rules.  If you have
-a really good need to break one of the rules herein, however, then it is
-best to ask on the B<rt-devel> mailing list first.
-
-Note that with much of this document, it is not so much the Right Way as
-it is Our Way.  We need to have conventions in order to make life easier
-for everyone.  So don't gripe, and just follow it, because you didn't
-get a good grade in "Plays Well With Others" in kindergarten and you
-want to make up for it now.
-
-If you have any questions, please ask us on the B<rt-devel> mailing list:
-
-        http://www.bestpractical.com/rt/lists.html
-
-We don't always follow this guide.  We are making changes throughout
-our code to be in line with it.  But just because we didn't do
-it yet, that is no excuse.  Do it anyway.  :-)
-
-This document is subject to change at the whims of the core RT team.
-We hope to add any significant changes at the bottom of the document.
-
-
-=head1 CODING PRINCIPLES
-
-=head2 Perl Version
-
-We code everything to Perl 5.10.1 or higher.
-
-=head2 Documentation
-
-All modules will be documented using the POD examples in the module
-boilerplate.  The function, purpose, use of the module will be
-explained, and each public API will be documented with name,
-description, inputs, outputs, side effects, etc.
-
-If an array or hash reference is returned, document the size of the
-array (including what each element is, as appropriate) and name each key
-in the hash.  For complex data structures, map out the structure as
-appropriate (e.g., name each field returned for each column from a DB
-call; yes, this means you shouldn't use "SELECT *", which you shouldn't
-use anyway).
-
-Also document what kind of data returned values are.  Is it an integer,
-a block of HTML, a boolean?
-
-All command-line program options will be documented using the
-boilerplate code for command-line programs, which doesn't yet exist.
-Each available function, switch, etc. should be documented, along
-with a statement of function, purpose, use of the program.  Do not
-use the same options as another program, for a different purpose.
-
-All web templates should be documented with a statement of function,
-purpose, and use in a mason comment block.
-
-Any external documents, and documentation for command-line programs and
-modules, should be written in POD, where appropriate. From there, they
-can be translated to many formats with the various pod2* translators.
-Read the perlpod manpage before writing any POD, because although POD is
-not difficult, it is not what most people are used to.  It is not a
-regular markup language; it is just a way to make easy documentation
-for translating to other formats.  Read, and understand, the perlpod
-manpage, and ask us or someone else who knows if you have any questions.
-
-
-=head2 Version
-
-Our distribution versions use tuples, where the first number is the
-major revision, the second number is the version, and third
-number is the subversion.  Odd-numbered versions are development
-versions.  Examples:
-
-        1.0.0           First release of RT 1
-        1.0.1           Second release of RT 1.0
-        1.0.10          etc.
-        1.1.0           First development release of RT 1.2 (or 2.0)
-        2.0.0           First release of RT 2
-
-Versions may end in "rc" and a number if they are release candidates:
-
-        2.0.0rc1        First release candiate for real 2.0.0
-
-
-=head2 Comments
-
-All code should be self-documenting as much as possible.  Only include
-necessary comments.  Use names like "$ticket_count", so you don't need to
-do something like:
-
-        # ticket count
-        my $tc = 0;
-
-Include any comments that are, or might be, necessary in order for
-someone else to understand the code.  Sometimes a simple one-line
-comment is good to explain what the purpose of the following code is
-for.  Sometimes each line needs to be commented because of a complex
-algorithm.  Read Kernighan & Pike's I<Practice of Programming> about
-commenting.  Good stuff, Maynard.
-
-
-=head2 Warnings and Strict
-
-All code must compile and run cleanly with "use strict" enabled and the
-perl "-w" (warnings) option on.  If you must do something that -w or
-strict complains about, there are workarounds, but the chances that you
-really need to do it that way are remote.
-
-=head2 Lexical Variables
-
-Use only lexical variables, except for special global variables
-($VERSION, %ENV, @ISA, $!, etc.) or very special circumstances (see
-%HTML::Mason::Commands::session ).  Global variables
-for regular use are never appropriate.  When necessary, "declare"
-globals with "use vars" or "our()".
-
-A lexical variable is created with my().  A global variable is
-pre-existing (if it is a special variable), or it pops into existence
-when it is used.  local() is used to tell perl to assign a temporary
-value to a variable.  This should only be used with special variables,
-like $/, or in special circumstances.  If you must assign to any global
-variable, consider whether or not you should use local().
-
-local() may also be used on elements of arrays and hashes, though there
-is seldom a need to do it, and you shouldn't.
-
-
-=head2 Pass by Reference
-
-Arrays and hashes should be passed to and from functions by reference
-only.  Note that a list and an array are NOT the same thing.  This
-is perfectly fine:
-
-        return($user, $form, $constants);
-
-An exception might be a temporary array of discrete arguments:
-
-        my @return = ($user, $form);
-        push @return, $constants if $flag;
-        return @return;
-
-Although, usually, this is better (faster, easier to read, etc.):
-
-        if ($flag) {
-                return($user, $form, $constants);
-        } else {
-                return($user, $form);
-        }
-
-We need to talk about Class::ReturnValue here.
-
-
-=head2 Method parameters
-
-If a method takes exactly one mandatory argument, the argument should be
-passed in a straightforward manner:
-
-        my $self = shift;
-        my $id = shift;
-
-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,
-            @_
-        );
-
-You may specify defaults to those named parameters instead of using
-C<undef> above, as long as it is documented as such.
-
-It is worth noting that the existing RT codebase had not followed this
-style perfectly; we are trying to fix it without breaking existing APIs.
-
-=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
-
-Always report errors using $RT::Logger. It's a Log::Dispatch object.
-Unlike message meant for the user, log messages are not to be
-internationalized.
-
-There are several different levels ($RT::Logger methods) of logging:
-
-=over 4
-
-=item debug
-
-Used for messages only needed during system debugging.
-
-=item info
-
-Should be used to describe "system-critical" events which aren't errors.
-Examples: creating users, deleting users, creating tickets, creating queues,
-sending email (message id, time, recipients), recieving mail, changing
-passwords, changing access control, superuser logins)
-
-=item error
-
-Used for RT-generated failures during execution.
-
-=item crit
-
-Should be used for messages when an action can not be completed due to some
-error condition beyond our control.
-
-=back
-
-In the web UI and modules, never print directly to STDERR.  Do not print
-directly to STDOUT, unless you need to print directly to the user's console.
-
-In command-line programs, feel free to print to STDERR and STDOUT as
-needed for direct console communication. But for actual error reporting,
-use the logging API.
-
-
-=head2 System Calls
-
-Always check return values from system calls, including open(),
-close(), mkdir(), or anything else that talks directly to the system.
-Perl built-in system calls return the error in $!; some functions in
-modules might return an error in $@ or some other way, so read the module's
-documentation if you don't know.  Always do something, even if it is
-just calling $RT::Logger->warning(), when the return value is not what you'd expect.
-
-
-
-=head1 STYLE
-
-Much of the style section is taken from the perlsyle manpage.  We make
-some changes to it here, but it wouldn't be a bad idea to read that
-document, too.
-
-=head2 Terminology
-
-=over 4
-
-=item function vs. sub(routine) vs. method
-
-Just because it is the Perl Way (not necessarily right for all
-languages, but the documented terminology in the perl documentation),
-"method" should be used only to refer to a subroutine that are object
-methods or class methods; that is, these are functions that are used
-with OOP that always take either an object or a class as the first
-argument. Regular subroutines, ones that are not object or class
-methods, are functions.  Class methods that create and return an object
-are optionally called constructors.
-
-=item Users
-
-"users" are normally users of RT, the ones hitting the site; if using
-it in any other context, specify.
-"system users" are user
-names on the operating system.  "database users" are the user names in
-the database server.  None of these needs to be capitalized.
-
-=back
-
-
-=head2 Names
-
-Don't use single-character variables, except as iterator variables.
-
-Don't use two-character variables just to spite us over the above rule.
-
-Constants are in all caps; these are variables whose value will I<never>
-change during the course of the program.
-
-        $Minimum = 10;          # wrong
-        $MAXIMUM = 50;          # right
-
-Other variables are lowercase, with underscores separating the words.
-They words used should, in general, form a noun (usually singular),
-unless the variable is a flag used to denote some action that should be
-taken, in which case they should be verbs (or gerunds, as appropriate)
-describing that action.
-
-        $thisVar      = 'foo';  # wrong
-        $this_var     = 'foo';  # right
-        $work_hard    = 1;      # right, verb, boolean flag
-        $running_fast = 0;      # right, gerund, boolean flag
-
-Arrays and hashes should be plural nouns, whether as regular arrays and
-hashes or array and hash references.  Do not name references with "ref"
-or the data type in the name.
-
-        @stories     = (1, 2, 3);      # right
-        $comment_ref = [4, 5, 6];      # wrong
-        $comments    = [4, 5, 6];      # right
-        $comment     = $comments->[0]; # right
-
-Make the name descriptive.  Don't use variables like "$sc" when you
-could call it "$story_count".  See L<"Comments">.
-
-There are several variables in RT that are used throughout the code,
-that you should use in your code.  Do not use these variable names for
-anything other than how they are normally used, and do not use any
-other variable names in their place.  Some of these are:
-
-        $self           # first named argument in object method
-
-Subroutines (except for special cases, like AUTOLOAD and simple accessors)
-begin with a verb, with words following to complete the action.  Accessors
-don't start with "Get" if they're just the name of the attribute.
-
-Accessors which return an object should end with the suffix Obj.
-
-This section needs clarification for RT.
-
-Words begin with a capital letter.  They
-should as clearly as possible describe the activity to be peformed, and
-the data to be returned.
-
-
-
-        Load();         # good
-        LoadByName();   # good
-        LoadById();             # good
-
-Subroutines beginning with C<_> are special: they are not to be used
-outside the current object.  There is not to be enforced by the code
-itself, but by someone very big and very scary.
-
-For large for() loops, do not use $_, but name the variable.
-Do not use $_ (or assume it) except for when it is absolutely
-clear what is going on, or when it is required (such as with
-map() and grep()).
-
-        for (@list) {
-            print;                      # OK; everyone knows this one
-            print uc;                   # wrong; few people know this
-            print uc $_;                # better
-        }
-
-Note that the special variable C<_> I<should> be used when possible.
-It is a placeholder that can be passed to stat() and the file test
-operators, that saves perl a trip to re-stat the file.  In the
-example below, using C<$file> over for each file test, instead of
-C<_> for subsequent uses, is a performance hit.  You should be
-careful that the last-tested file is what you think it is, though.
-
-        if (-d $file) {         # $file is a directory
-            # ...
-        } elsif (-l _) {        # $file is a symlink
-            # ...
-        }
-
-Package names begin with a capital letter in each word, followed by
-lower case letters (for the most part).  Multiple words should be StudlyCapped.
-
-        RT::User                        # good
-        RT::Database::MySQL             # proper name
-        RT::Display::Provider           # good
-        RT::CustomField                 # not so good, but OK
-
-Plugin modules should begin with "RT::Extension::", followed by the name
-of the plugin.
-
-=head1 Code formatting
-
-When in doubt, use perltidy; RT includes a F<.perltidyrc>.
-
-=head2 Indents and Blank Space
-
-All indents should be four spaces; hard tabs are forbidden.
-
-No space before a semicolon that closes a statement.
-
-        foo(@bar) ;     # wrong
-        foo(@bar);      # right
-
-Line up corresponding items vertically.
-
-        my $foo   = 1;
-        my $bar   = 2;
-        my $xyzzy = 3;
-
-        open(FILE, $fh)   or die $!;
-        open(FILE2, $fh2) or die $!;
-
-        $rot13 =~ tr[abcedfghijklmnopqrstuvwxyz]
-                    [nopqrstuvwxyzabcdefghijklm];
-
-        # note we use a-mn-z instead of a-z,
-        # for readability
-        $rot13 =~ tr[a-mn-z]
-                    [n-za-m];
-
-Put blank lines between groups of code that do different things.  Put
-blank lines after your variable declarations.  Put a blank line before a
-final return() statement.  Put a blank line following a block (and
-before, with the exception of comment lines).
-
-An example:
-
-        # this is my function!
-        sub foo {
-            my $val = shift;
-            my $obj = new Constructor;
-            my($var1, $var2);
-
-            $obj->SetFoo($val);
-            $var1 = $obj->Foo();
-
-            return($val);
-        }
-
-        print 1;
-
-
-=head2 Parentheses
-
-For control structures, there is a space between the keyword and opening
-parenthesis.  For functions, there is not.
-
-        for(@list)      # wrong
-        for (@list)     # right
-
-        my ($ref)       # wrong
-        my($ref)        # right
-
-Be careful about list vs. scalar context with parentheses!
-
-        my @array = ('a', 'b', 'c');
-        my($first_element) = @array;            # a
-        my($first_element) = ('a', 'b', 'c');   # a
-        my $element_count  = @array;            # 3
-        my $last_element   = ('a', 'b', 'c');   # c
-
-Always include parentheses after functions, even if there are no arguments.
-There are some exceptions, such as list operators (like print) and unary
-operators (like undef, delete, uc).
-
-There is no space inside the parentheses, unless it is needed for
-readability.
-
-        for ( map { [ $_, 1 ] } @list ) # OK
-        for ( @list )                   # not really OK, not horrible
-
-On multi-line expressions, match up the closing parenthesis with either
-the opening statement, or the opening parenthesis, whichever works best.
-Examples:
-
-        @list = qw(
-            bar
-            baz
-        );                      # right
-
-        if ($foo && $bar && $baz
-                 && $buz && $xyzzy) {
-            print $foo;
-        }
-
-Whether or not there is space following a closing parenthesis is
-dependent on what it is that follows.
-
-        print foo(@bar), baz(@buz) if $xyzzy;
-
-Note also that parentheses around single-statement control expressions,
-as in C<if $xyzzy>, are optional (and discouraged) C<if> it is I<absolutely>
-clear -- to a programmer -- what is going on.  There is absolutely no
-need for parentheses around C<$xyzzy> above, so leaving them out enhances
-readability.  Use your best discretion.  Better to include them, if
-there is any question.
-
-The same essentially goes for perl's built-in functions, when there is
-nothing confusing about what is going on (for example, there is only one
-function call in the statement, or the function call is separated by a
-flow control operator).  User-supplied functions must always include
-parentheses.
-
-        print 1, 2, 3;                          # good
-        delete $hash{key} if isAnon($uid);      # good
-
-
-However, if there is any possible confusion at all, then include the
-parentheses.  Remember the words of Larry Wall in the perlstyle manpage:
-
-        When in doubt, parenthesize.  At the very least it will
-        let some poor schmuck bounce on the % key in vi.
-
-        Even if you aren't in doubt, consider the mental welfare
-        of the person who has to maintain the code after you, and
-        who will probably put parens in the wrong place.
-
-So leave them out when it is absoutely clear to a programmer, but if
-there is any question, leave them in.
-
-
-=head2 Braces
-
-(This is about control braces, not hash/data structure braces.)
-
-There is always a space befor the opening brace.
-
-        while (<$fh>){  # wrong
-        while (<$fh>) { # right
-
-A one-line block may be put on one line, and the semicolon may be
-omitted.
-
-        for (@list) { print }
-
-Otherwise, finish each statement with a semicolon, put the keyword and
-opening curly on the first line, and the ending curly lined up with the
-keyword at the end.
-
-        for (@list) {
-            print;
-            smell();
-        }
-
-Generally, we prefer "cuddled elses":
-
-        if ($foo) {
-            print;
-        } else {
-            die;
-        }
-
-=head2 Operators
-
-Put space around most operators.  The primary exception is the for
-aesthetics; e.g., sometimes the space around "**" is ommitted,
-and there is never a space before a ",", but always after.
-
-        print $x , $y;  # wrong
-        print $x, $y;   # right
-
-        $x = 2 >> 1;    # good
-        $y = 2**2;      # ok
-
-Note that "&&" and "||" have a higher precedence than "and" and "or".
-Other than that, they are exactly the same.  It is best to use the lower
-precedence version for control, and the higher for testing/returning
-values.  Examples:
-
-        $bool = $flag1 or $flag2;       # WRONG (doesn't work)
-        $value = $foo || $bar;          # right
-        open(FILE, $file) or die $!;
-
-        $true  = foo($bar) && baz($buz);
-        foo($bar) and baz($buz);
-
-Note that "and" is seldom ever used, because the statement above is
-better written using "if":
-
-        baz($buz) if foo($bar);
-
-Most of the time, the confusion between and/&&, or/|| can be alleviated
-by using parentheses.  If you want to leave off the parentheses then you
-I<must> use the proper operator.  But if you use parentheses -- and
-normally, you should, if there is any question at all -- then it doesn't
-matter which you use.  Use whichever is most readable and aesthetically
-pleasing to you at the time, and be consistent within your block of code.
-
-Break long lines AFTER operators, except for ".", "and", "or", "&&", "||".
-Try to keep the two parts to a binary operator (an operator that
-has two operands) together when possible.
-
-        print "foo" . "bar" . "baz" .
-              "buz";                            # wrong
-
-        print "foo" . "bar" . "baz"
-            . "buz";                            # right
-
-        print $foo unless $x == 3 && $y ==
-                4 && $z == 5;                   # wrong
-
-        print $foo unless $x == 3 && $y == 4
-                       && $z == 5;              # right
-
-
-=head2 Other
-
-Put space around a complex subscript inside the brackets or braces.
-
-        $foo{$bar{baz}{buz}};   # OK
-        $foo{ $bar{baz}{buz} }; # better
-
-In general, use single-quotes around literals, and double-quotes
-when the text needs to be interpolated.
-
-It is OK to omit quotes around names in braces and when using
-the => operator, but be careful not to use a name that doubles as
-a function; in that case, quote.
-
-        $what{'time'}{it}{is} = time();
-
-When making compound statements, put the primary action first.
-
-        open(FILE, $fh) or die $!;      # right
-        die $! unless open(FILE, $fh);  # wrong
-
-        print "Starting\n" if $verbose; # right
-        $verbose && print "Starting\n"; # wrong
-
-
-Use here-docs instead of repeated print statements.
-
-                print <<EOT;
-        This is a whole bunch of text.
-        I like it.  I don't need to worry about messing
-        with lots of print statements and lining them up.
-        EOT
-
-Just remember that unless you put single quotes around your here-doc
-token (<<'EOT'), the text will be interpolated, so escape any "$" or "@"
-as needed.
-
-=head1 INTERNATIONALIZATION
-
-
-=head2 String extraction styleguide
-
-=over 4
-
-=item Web templates
-
-Templates should use the /l filtering component to call the localisation
-framework
-
-The string              Foo!
-
-Should become           <&|/l&>Foo!</&>
-
-All newlines should be removed from localized strings, to make it easy to
-grep the codebase for strings to be localized
-
-The string              Foo
-                        Bar
-                        Baz
-
-Should become           <&|/l&>Foo Bar Baz</&>
-
-
-Variable subsititutions should be moved to Locale::MakeText format
-
-The string              Hello, <%$name %>
-
-should become           <&|/l, $name &>Hello, [_1]</&>
-
-
-Multiple variables work just like single variables
-
-The string              You found <%$num%> tickets in queue <%$queue%>
-
-should become           <&|/l, $num, $queue &>You found [_1] tickets in queue [_2]</&>
-
-When subcomponents are called in the middle of a phrase, they need to be escaped
-too:
-
-The string               <input type="submit" value="New ticket in">&nbsp<& /Elements/SelectNewTicketQueue&>
-
-should become           <&|/l, $m->scomp('/Elements/SelectNewTicketQueue')&><input type="submit" value="New ticket in"> [_1]</&>
-
-
-
-
-The string      <& /Widgets/TitleBoxStart, width=> "40%", titleright => "RT $RT::VERSION for   RT->Config->Get('rtname')", title => 'Login' &>
-
-should become   <& /Widgets/TitleBoxStart,
-                        width=> "40%",
-                        titleright => loc("RT [_1] for [_2]",$RT::VERSION, RT->Config->Get('rtname')),
-                        title => loc('Login'),
-                &>
-
-=item Library code
-
-
-
-Within RT's core code, every module has a localization handle available through the 'loc' method:
-
-The code        return ( $id, "Queue created" );
-
-should become   return ( $id, $self->loc("Queue created") );
-
-When returning or localizing a single string, the "extra" set of parenthesis () should be omitted.
-
-The code        return ("Subject changed to ". $self->Data );
-
-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
-
-=head1 CODING PRCEDURE
-
-This is for new programs, modules, specific APIs, or anything else.
-
-=over 4
-
-=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.
-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 rt-devel
-
-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.).
-
-The best way to do this is to take the documentation portion of the
-boilerplate and fill it in.  You can make changes later if necessary,
-but fill it in as much as you can.
-
-
-
-=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
-belittle you (that's just a nice side effect), it is to make sure that
-you understand your code, that we understand your code, that it won't
-break other code, that it follows the documentation and existing
-proposal.  It is to check for possible optimizations or better ways of
-doing it.
-
-Note that all code is expected to follow the coding principles and style
-guide contained in this document.
-
-
-=item Finish it up
-
-After the code is done (possibly going through multiple code reviews),
-if you do not have repository access, submit it to rt-bugs at fsck.com as a
-unified diff. From that point on, it'll be handled by someone with
-repository access.
-
-=back
-
-
-=head1 BUG REPORTS, PATCHES
-
-Use rt-bugs at bestpractical.com for I<any> bug that is not being fixed
-immediately.  If it is not in RT, there is a good chance it will not be
-dealt with.
-
-Send patches to rt-bugs at bestpractical.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
-
-Talk about DBIx::SearchBuilder
-
-Talk about mason
-        component style
-        cascading style sheets
-
-Talk about adding a new translation
-
-Talk more about logging

commit 25b8cdcd3af72efb8f3c9e719c36dd03f2932bba
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue May 5 00:06:19 2015 -0400

    Move code conventions and I18N later, below how to actually get the code

diff --git a/docs/hacking.pod b/docs/hacking.pod
index 6ba056e..4cf655c 100644
--- a/docs/hacking.pod
+++ b/docs/hacking.pod
@@ -49,152 +49,6 @@ messages explain the B<why> as much as the B<what> of each commit, and
 not include extraneous changes.
 
 
-=head1 Code conventions
-
-The most salient convention is that RT uses four spaces for indentation,
-and eschews tabs entirely; a F<.perltidyrc> is included for use with
-L<perltidy|https://metacpan.org/pod/distribution/Perl-Tidy/bin/perltidy>.
-
-Note that the RT codebase is more than ten years old; as such, there are
-sections which do not (yet) conform to the perltidyrc.  Please attempt
-to follow its guidelines, even if the code surrounding your changes does
-not yet.  Better yet, include a commit which fixes the style of the
-surrounding code first.
-
-=head1 Internationalization
-
-RT has been translated into several dozen languages. We use Launchpad
-( https://translations.launchpad.net/rt ) to crowdsource our
-translations into C<po> files. RT uses L<Locale::Maketext> to
-localize its user interface.
-
-Your first stop on this magical journey of internationalization
-is L<Locale::Maketext::TPJ13>, which explains the whys of
-L<Locale::Maketext>. RT uses most of the features developed in that
-article.
-
-Strings that are displayed to users should be passed through the
-C<loc("...")> function or the C<< <&|/l&>...</&> >> Mason template.
-C<loc> and C</l> both take parameters, which are used in place of
-string interpolation (much like C<sprintf>). It's acceptable to use
-HTML in C</l> calls, especially for bold and emphasis. However, you
-should limit the amount of HTML that translators must keep exactly
-correct, which means avoid including tags that wrap the entire
-translatable string, especially C<< <p> >>.
-
-    <p><&|/l, $button &>Do <em>not</em> click [_1]</&></p> # ok
-
-    <&|/l, $button &><p>Do <em>not</em> click [_1]</p></&> # not ok
-
-In a few places in RT we also pass HTML as parameters to C<loc()>
-so that translators do not have to reproduce it exactly, and we can
-also change it more freely. For example:
-
-    <&|/l,
-        '<a href="http://www.gnu.org/licenses/gpl-2.0.html">',
-        '</a>',
-    &>Distributed under [_1]version 2 of the GNU GPL[_2].</&>
-
-F<devel/tools/extract-message-catalog> looks for C<loc("...")> and
-C<< <&|/l&>...</&> >> in our source code to pick out translatable
-strings, clean them up, and put them into F<share/po> files. We use
-our C<.po> files not only to populate L<Locale::Maketext>'s lexicons,
-but also to sync new translatable strings and translations with
-Launchpad. This Launchpad sync is typically done early during the
-freeze of RC releases to give our volunteer translators time to
-translate all the new strings which, because of the RC freeze, won't
-continue changing.
-
-Because C<loc()> and C</l> are used to generate strings for human
-eyes, they generally must be used "close to the browser". These are
-directly in Mason templates, or in functions that return text that
-will be passed through Mason. However, in many places in RT we have
-hardcoded strings which need translations. For example, the C<$RIGHTS>
-hash in F<lib/RT/Queue.pm> maps rights' names (which must be
-translatable) to their descriptions (which also must be translatable).
-However, when we're declaring such structures, we do not want to
-translate them straight away. RT uses English internally, including
-in its web forms, so we do not want to localize rights' names except
-for display, otherwise things might break weirdly when you check
-if a user has the "Superusuario" right. Furthermore, when we're
-declaring such data structures at compile time, there is no current
-user to select which language to use for localization. Thus, we
-cannot call C<loc()> when declaring C<$RIGHTS> and other similar
-places.
-
-For this reason, F<devel/tools/extract-message-catalog> lets you
-denote translatable strings with comments. That's what the C<#loc_pair>
-comments in the C<$RIGHTS> hash in F<lib/RT/Queue.pm> indicate.
-Since we have those comments, our toolchain will put the rights'
-names and descriptions into F<share/po> files, which enables
-translation by our lovely volunteers. Later on, when RT displays
-information about rights in the web UI, we'll pass the right's name
-through C<loc>, and L<Locale::Maketext> will then be able to find
-our "Superusuario". So although we never used a literal
-C<loc("SuperUser")>, we still get its effects thanks to the
-C<#loc_pair> comments and using C<loc($RightName)>.
-
-C<#loc_pair> is used for declaring that the both the key and value
-of a particular C<< key => value >> pair are translatable. There
-are other markers that you can use.
-
-C<#loc> is used for declaring that a particular string is translatable.
-Its parsing is pretty strict so you can use it to declare that only
-the value of a particular C<< key => value >> pair is translatable.
-
-C<#loc_left_pair> is used for declaring that the I<key> of a
-particular C<< key => value >> pair is translatable. This is of
-very limited usefulness.
-
-C<#loc_right_pair> does NOT exist. C<#loc> works in such cases since its
-parser does not extend beyond the string at the end of a line.  However,
-if the string is I<not> at the end of the line, C<#loc{word}> declares
-that the value associated with the key I<word> (earlier on the same
-line) is to be loc'd.  This is useful for inline hashes:
-
-    # Note the string "baz" is to be loc'd
-    foo => { bar => "baz", troz => "zort" },  # loc{bar}
-
-=head1 Development tips
-
-=head2 Setting up a development environment
-
-=head2 Test suite
-
-RT also comes with a fairly complete test suite.  To run it, you will
-need to set environment variables to a database user and password which
-can create and drop databases:
-
-    export RT_DBA_USER=root
-    export RT_DBA_PASSWORD=
-
-You'll need to configure RT and make sure you have all the dependencies
-before running tests.  To do this in place without installing:
-
-    ./configure.ac --with-my-user-group --enable-layout=inplace --enable-developer
-    make testdeps
-    make fixdeps
-
-Adjust the relevant database options as necessary if you want to test on
-Postgres, Oracle, or SQLite.  The default is MySQL.
-
-To run the test suite:
-
-    make test
-
-If you have multiple processors, you can run the test suite in parallel,
-which will be significantly faster:
-
-    make test-parallel
-
-The C<*-trunk> and C<master> branches are expected to always be passing
-all tests.  While it is acceptable to break tests in an intermediate
-commit, a branch which does not pass tests will not be merged.  Ideally,
-commits which fix a bug should also include a testcase which fails
-before the fix and succeeds after.
-
-
-
 =head1 git quickstart
 
 =over
@@ -299,4 +153,156 @@ send it to C<rt-devel at bestpractical.com>.
 If you have another bug or feature to implement, simply restart the
 process at step 4.
 
+=head1 Code conventions
+
+The most salient convention is that RT uses four spaces for indentation,
+and eschews tabs entirely; a F<.perltidyrc> is included for use with
+L<perltidy|https://metacpan.org/pod/distribution/Perl-Tidy/bin/perltidy>.
+
+Note that the RT codebase is more than ten years old; as such, there are
+sections which do not (yet) conform to the perltidyrc.  Please attempt
+to follow its guidelines, even if the code surrounding your changes does
+not yet.  Better yet, include a commit which fixes the style of the
+surrounding code first.
+
+=head1 Running the test suite
+
+RT also comes with a fairly complete test suite.  To run it, you will
+need to set environment variables to a database user and password which
+can create and drop databases:
+
+    export RT_DBA_USER=root
+    export RT_DBA_PASSWORD=
+
+You'll need to configure RT and make sure you have all the dependencies
+before running tests.  To do this in place without installing:
+
+    ./configure.ac --with-my-user-group --enable-layout=inplace --enable-developer
+    make testdeps
+    make fixdeps
+
+Adjust the relevant database options as necessary if you want to test on
+Postgres, Oracle, or SQLite.  The default is MySQL.
+
+To run the test suite:
+
+    make test
+
+If you have multiple processors, you can run the test suite in parallel,
+which will be significantly faster:
+
+    make test-parallel
+
+The C<*-trunk> and C<master> branches are expected to always be passing
+all tests.  While it is acceptable to break tests in an intermediate
+commit, a branch which does not pass tests will not be merged.  Ideally,
+commits which fix a bug should also include a testcase which fails
+before the fix and succeeds after.
+
+=head1 Internationalization
+
+RT has been translated into several dozen languages. We use
+L<Launchpad|https://translations.launchpad.net/rt> to crowdsource our
+translations into C<po> files. RT uses L<Locale::Maketext> to localize
+its user interface.
+
+Your first stop on this magical journey of internationalization
+is L<Locale::Maketext::TPJ13>, which explains the whys of
+L<Locale::Maketext>. RT uses most of the features developed in that
+article.
+
+Strings that are displayed to users should be passed through the
+C<loc("...")> function or the C<< <&|/l&>...</&> >> Mason template.
+C<loc> and C</l> both take parameters, which are used in place of
+string interpolation (much like C<sprintf>). It's acceptable to use
+HTML in C</l> calls, especially for bold and emphasis. However, you
+should limit the amount of HTML that translators must keep exactly
+correct, which means avoid including tags that wrap the entire
+translatable string, especially C<< <p> >>.
+
+    <p><&|/l, $button &>Do <em>not</em> click [_1]</&></p> # ok
+
+    <&|/l, $button &><p>Do <em>not</em> click [_1]</p></&> # not ok
+
+In a few places in RT we also pass HTML as parameters to C<loc()>
+so that translators do not have to reproduce it exactly, and we can
+also change it more freely. For example:
+
+    <&|/l,
+        '<a href="http://www.gnu.org/licenses/gpl-2.0.html">',
+        '</a>',
+    &>Distributed under [_1]version 2 of the GNU GPL[_2].</&>
+
+F<devel/tools/extract-message-catalog> looks for C<loc("...")> and
+C<< <&|/l&>...</&> >> in our source code to pick out translatable
+strings, clean them up, and put them into F<share/po> files. We use
+our C<.po> files not only to populate L<Locale::Maketext>'s lexicons,
+but also to sync new translatable strings and translations with
+Launchpad. This Launchpad sync is typically done early during the
+freeze of RC releases to give our volunteer translators time to
+translate all the new strings which, because of the RC freeze, won't
+continue changing.
+
+Because C<loc()> and C</l> are used to generate strings for human
+eyes, they generally must be used "close to the browser". These are
+directly in Mason templates, or in functions that return text that
+will be passed through Mason. However, in many places in RT we have
+hardcoded strings which need translations. For example, the C<$RIGHTS>
+hash in F<lib/RT/Queue.pm> maps rights' names (which must be
+translatable) to their descriptions (which also must be translatable).
+However, when we're declaring such structures, we do not want to
+translate them straight away. RT uses English internally, including
+in its web forms, so we do not want to localize rights' names except
+for display, otherwise things might break weirdly when you check
+if a user has the "Superusuario" right. Furthermore, when we're
+declaring such data structures at compile time, there is no current
+user to select which language to use for localization. Thus, we
+cannot call C<loc()> when declaring C<$RIGHTS> and other similar
+places.
+
+For this reason, F<devel/tools/extract-message-catalog> lets you
+denote translatable strings with comments. That's what the C<#loc_pair>
+comments in the C<$RIGHTS> hash in F<lib/RT/Queue.pm> indicate.
+Since we have those comments, our toolchain will put the rights'
+names and descriptions into F<share/po> files, which enables
+translation by our lovely volunteers. Later on, when RT displays
+information about rights in the web UI, we'll pass the right's name
+through C<loc>, and L<Locale::Maketext> will then be able to find
+our "Superusuario". So although we never used a literal
+C<loc("SuperUser")>, we still get its effects thanks to the
+C<#loc_pair> comments and using C<loc($RightName)>.
+
+The following comments are noticed by F<extract-message-catalog>:
+
+=over
+
+=item C<#loc>
+
+Used for declaring that a particular string is translatable.  Its
+parsing is pretty strict so you can use it to declare that only the
+value of a particular C<< key => value >> pair is translatable.
+
+=item C<#loc_pair>
+
+Used for declaring that the both the key and value of a particular C<<
+key => value >> pair are translatable.
+
+=item C<#loc_left_pair>
+
+Used for declaring that the I<key> of a particular C<< key => value >>
+pair is translatable. This is generally of limited usefulness.
+
+=item C<#loc{word}>
+
+There is no need for a C<#loc_right_pair>, as C<#loc> works in such
+cases.  However, if the string is I<not> at the end of the line,
+C<#loc{word}> declares that the value associated with the key I<word>
+(earlier on the same line) is to be loc'd.  This is useful for inline
+hashes:
+
+    # Note the string "baz" is to be loc'd
+    foo => { bar => "baz", troz => "zort" },  # loc{bar}
+
+=back
+
 =cut

commit 927ab0636c7f470e1ced75d5b0358df6d98b9091
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue May 5 00:09:09 2015 -0400

    Fix example `git config` incant

diff --git a/docs/hacking.pod b/docs/hacking.pod
index 4cf655c..bb30828 100644
--- a/docs/hacking.pod
+++ b/docs/hacking.pod
@@ -72,7 +72,7 @@ Configure git to know your name and email address; git uses these when
 it makes commits.
 
     git config user.email your.email at example.com
-    git config user.name Examp L. Name
+    git config user.name "Examp L. Name"
 
 =item 4.
 

commit c38347e9000bc405c7b44b74c77f6d06488fbf56
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue May 5 00:09:24 2015 -0400

    Update example versions to be more modern

diff --git a/docs/hacking.pod b/docs/hacking.pod
index bb30828..5adcbe9 100644
--- a/docs/hacking.pod
+++ b/docs/hacking.pod
@@ -77,12 +77,12 @@ it makes commits.
 =item 4.
 
 Switch to the appropriate point to base your work on; this is generally
-C<origin/> followed by the major version, followed by C<-trunk>.  For
-example, if your bug was observed in version 3.8.9, you would choose
-C<origin/3.8-trunk>; if it was in 4.0.0, you would choose
-C<origin/4.0-trunk>.  New features should be based on C<origin/master>.
+C<origin/>, followed by the major version, followed by C<-trunk>.  For
+example, if your bug was observed in version 4.0.10, you would choose
+C<origin/4.0-trunk>; if it was in 4.2.2, you would choose
+C<origin/4.2-trunk>.  New features should be based on C<master>.
 
-    git checkout --track origin/4.0-trunk
+    git checkout --track origin/4.2-trunk
 
 =item 5.
 

commit 2c0b42c2e6dc39e5cfb0b4310713e7d969d4cb1a
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue May 5 00:10:12 2015 -0400

    Gentle the introduction, provide more pointers, and note that pull requests are fine

diff --git a/docs/hacking.pod b/docs/hacking.pod
index 5adcbe9..2de4f3c 100644
--- a/docs/hacking.pod
+++ b/docs/hacking.pod
@@ -1,16 +1,35 @@
-=head1 Development of RT
+=pod
 
-RT's source code is stored in a C<git> repository.  If you are not
-familiar with git, see L</git quickstart>, below, for a short tutorial
-which will give you enough information to get started submitting patches
-to RT.
+RT is open source, and we welcome contributions from the community.
+This document details the how of getting your changes into RT.
+
+=head1 Version control
+
+We use L<Git|http://git-scm.com/> to track all source code
+changes. L<GitHub|https://github.com/bestpractical> provides a
+L<user-friendly view of the
+repository|https://github.com/bestpractical/rt>.
+
+You can check out a copy of the stable trunk of RT (stable) from Git
+using the command line:
+
+    git clone git://github.com/bestpractical/rt.git -b stable
+
+If you're feeling adventurous, check out a copy of the development
+trunk of RT (master) from Git using:
+
+    git clone git://github.com/bestpractical/rt.git -b master
+
+You can submit changes either via mail to the L<C<rt-devel> mailing
+list|https://bestpractical.com/rt/lists.html>, or via pull requests on
+GitHub.  If you are not familiar with git, see L</git quickstart>,
+below, for a short tutorial which will give you enough information to
+get started submitting patches to RT.
 
 The rest of this document details conventions and tips surrounding the
 organization of RT's version control, source code conventions, and how
 to submit patches.
 
-
-
 =head1 Organization of rt.git
 
 The RT source repository is available via git from GitHub; you can
@@ -51,6 +70,10 @@ not include extraneous changes.
 
 =head1 git quickstart
 
+You are welcome to submit changes as pull requests via
+L<GitHub|https://github.com/bestpractical/rt>.  If you're unfamiliar
+with git, the following guide is an alternative way to get started:
+
 =over
 
 =item 1.

commit bc28fe98fcde5a7446fe29c23798177c48e27b89
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue May 5 00:10:49 2015 -0400

    Explicitly note the license and contributing agreement

diff --git a/docs/hacking.pod b/docs/hacking.pod
index 2de4f3c..65bdf5c 100644
--- a/docs/hacking.pod
+++ b/docs/hacking.pod
@@ -1,7 +1,8 @@
 =pod
 
-RT is open source, and we welcome contributions from the community.
-This document details the how of getting your changes into RT.
+RT is L<open source|/License>, and we welcome contributions from the
+community.  This document details the how of getting your changes into
+RT.
 
 =head1 Version control
 
@@ -176,6 +177,37 @@ send it to C<rt-devel at bestpractical.com>.
 If you have another bug or feature to implement, simply restart the
 process at step 4.
 
+
+=head1 License
+
+RT is licensed under the L<GNU GPL, Version
+2|https://www.gnu.org/licenses/gpl-2.0.html>; it is copyright by Best
+Practical Solutions.
+
+RT does not have an explicit Contributor License Agreement requirement,
+or requirement of Developer Certificate of Origin (C<Signed-Off-By:>);
+rather, it has the following legal statement concerning contributions:
+
+=over
+
+(The following paragraph is not intended to limit the rights granted
+to you to modify and distribute this software under the terms of
+the GNU General Public License and is only of importance to you if
+you choose to contribute your changes and enhancements to the
+community by submitting them to Best Practical Solutions, LLC.)
+
+By intentionally submitting any modifications, corrections or
+derivatives to this work, or any other work intended for use with
+Request Tracker, to Best Practical Solutions, LLC, you confirm that
+you are the copyright holder for those contributions and you grant
+Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
+royalty-free, perpetual, license to use, copy, create derivative
+works based on those contributions, and sublicense and distribute
+those contributions and any derivatives thereof.
+
+=back
+
+
 =head1 Code conventions
 
 The most salient convention is that RT uses four spaces for indentation,

-----------------------------------------------------------------------


More information about the rt-commit mailing list