Coding Standards

Perl

Try to follow the style of the existing Bricolage code. Except where it is bad, of course. Basically, write in the style you see in most Perl books and documentation, particularly perlstyle and Perl Best Practices.

Although historically the Bricolage code has not enforced whitespace rules, we now request that you use 4-space indents (including for continued lines) and discourage the use of tabs. The following settings for some of the more popular editors are thus recommended while editing Bricolage source code.

Emacs

We strongly recommend that you use cperl-mode while editing Bricolage sources in Emacs. Grab the latest version from the CPAN, install it, and then place the following in your ~/.emacs file:

(setq-default indent-tabs-mode nil)
(custom-set-variables
 '(case-fold-search t)
 '(cperl-close-paren-offset -4)
 '(cperl-continued-statement-offset 4)
 '(cperl-indent-level 4)
 '(cperl-indent-parens-as-block t)
 '(cperl-tab-always-indent t)
 '(indent-tabs-mode nil))
 '(cperl-indent-parens-as-block t)
 '(cperl-close-paren-offset -4)

Also, if you’d like to take advantage of the full functionality of cperl-mode and have it automatically parse all Perl source files, add these settings, as well:

(defalias 'perl-mode 'cperl-mode)
(setq auto-mode-alist
      (append
       '(("\\.\\([pP]\\([Llm]\\|erl\\)\\|al\\|pod\\)\\'" . cperl-mode))
       auto-mode-alist))
  (setq cperl-hairy t)
  (setq interpreter-mode-alist (append interpreter-mode-alist
     '(("miniperl" . cperl-mode))))

When editing Bricolage Mason components, mmm-mode can help. Its Mason mode will parse Mason component files and use sgml-mode in HTML spaces and cperl-mode in Mason blocks. Grab it from SourceForge, install it, and then add the following to your ~/.emacs file to have it automatically parse your Bricolage Mason component files:

(add-to-list 'load-path "/usr/local/share/emacs/site-lisp")
(require 'mmm-mode)
(require 'mmm-mason)
(setq mmm-global-mode 'maybe)
(add-to-list 'auto-mode-alist '("/usr/local/bricolage/comp" . sgml-mode))
(mmm-add-mode-ext-class 'sgml-mode "/usr/local/bricolage/comp" 'mason)

And if you need to examine the Mason object files created by Bricolage in order to chase down bugs and such, you can use the cperl-mode in those files by adding this to your ~/.emacs file:

  (add-to-list 'auto-mode-alist '("/usr/local/bricolage/data/obj" . cperl-mode))

Vim

Rafael Garcia-Suarez has written a Vim indent macro which (for the most part) duplicates the behavior of Emacs perl-mode. As of Vim 6.0 it is included in the Vim distribution and should be found in $VIMRUNTIME/indent/perl. The easiest way to use it though is to place the following line in your ~/.vimrc file:

source $VIMRUNTIME/indent.vim

You’ll also need to add these lines.

set tabstop=8
set softtabstop=4
set shiftwidth=4
set expandtab

The first three lines make Vim duplicate the behavior of Emacs in creating the appearance of 4 space tabs with a mix of tabs and spaces. This is necessary for reading older Bricolage files which were written this way, and haven’t yet been re-tabbed.

The expandtab setting does the Right Thing under the new rules, in that it doesn’t use tabs at all, only spaces.

SQL

The standard for writing SQL in Bricolage is pretty straight-forward: format
the SQL in 78 columns or less, and use heredocs where possible. For example:

my $sql = "<<    END_SQL";
    SELECT id, name, description, publish_date, cover_date, current_version,
           publish_status, active
    FROM   story
    WHERE  id = ?
END_SQL

Also, when writing your queries, please follow the following rules with regard
to table aliases:

• Do not alias any table names in queries, unless those table names are longer than 8 characters, or are referred to in the FROM clause more than once. Under this rule, for example, “story” and “member” would never be aliased unless referenced twice.

• For table names over 8 characters, abbreviated aliases are acceptable provided that they are still long enough to be informative. i.e. “st_cat” instead of “sc” for “story_category”.

• For multiple links to the same table, use an alias which is long enough to be informative, such as “story2” or “inst_2” for the second reference to “story” or “story_instance”.

The reason for these rules is that single-letter aliases for queries are as
unreadable and unmaintainable as single-letter variable names. You get
halfway down the page, and you can no longer remember to what tables they
refer.

The rules above are actually taken from O’Reilly’s “Introduction to PL/SQL
Programming,” which has an excellent chapter on code cleanliness in SQL and
SQL-extension languages.

Other Good Practices

• Keep subroutines short. Each subroutine should handle one task. For example, if you have a subroutine get_foo, and getting “foo” requires getting “bar” and “qux”, then make two more subroutines get_bar and get_qux and put them in the “private functions” section. Also if you duplicate some code in several subroutines, factor it out into another subroutine. It’s easier to maintain one subroutine than to maintain three.

• Limit the width of your code to 78 characters if possible. This makes them easier to read. Keeping subroutines short helps to meet this goal, as well.

• Make enough comments so that someone maintaining your code can understand what is going on. Comments shouldn’t be redundant with the code. They should explain non-obvious code. Comments can be bad in some cases if code changes and the corresponding comment is not kept in sync. So keep the comments in sync!

• Use Perl idioms if it is clear and concise, but use them with care. Implicit variables can be slick, but hard to understand; add a comment if you use uncommon ones.

• Use Mason components only for display, and put business logic into library modules and callback classes.

• For POD, generally you copy/paste it from another module. Recently we are trying to cut out some of useless things like B<Side Effects:> NONE. For a canonical example, see Bric::Biz::Site.

• Avoid magic values, i.e. don’t hard-code numbers into code. It tends to become a maintenance problem if you put the number 1023 throughout a module and then somewhere else you have 1021; is 1021 related to 1023 (1023 – 2), or is it just another random number? And what does 1023 mean? Put a constant at the top of the module and use that, instead.

In the end, just try to follow the existing code style, and take into consideration any feedback you get from the mailing list when patches are submitted.

Testing

All Bricolage patches should be accompanied by the necessary tests. You are strongly encouraged to write comprehensive tests that thoroughly test whatever changes or additions you make to the Bricolage API. You are also strongly encouraged to write tests for the current API, if you find that adequate tests have not yet been written (and this is true in a great many places, unfortunately). Although Bricolage does not yet support UI tests, we take our API testing seriously, and so should you. Here’s what you need to know to write tests for Bricolage.

Bricolage contains a test suite based on Test::Class. The test classes live in the t/Bric directory, and all subclass Bric::Test::Base. If you’re familiar with Test::More, then the syntax for how the
Test::Class-based classes work should be pretty readily apparent. See the
Test::Class documentation for details. It’s definitely worth a
read. If you haven’t used Test::More, its documentation is also a must-read.

Bricolage has two different sets of tests, those run by make test and those run by make devtest. The former are stand-alone tests that don’t rely on the presence of a Bricolage database to be run. They can thus be run before make install. The idea here is that we offer a basic set of tests that can be run via the standard Perl installation pattern of

perl Makefile.PL
make
make test
make install

The tests run by make devtest are intended to be run during development. They require that a Bricolage database be installed and running. They will SELECT, INSERT, UPDATE, and DELETE data from that database, so make devtest should never be run against a production database. Indeed, you should in general have a clean, fresh database installation to run the tests against. Although the test suite makes every effort to clean up after itself by @DELETE@ing data it has added to the database, it is unfortunately imperfect, and extra data may be left.

So, once more, make devtest should never be run against a production database. ’Nuff said.

You can get a clean database without reinstalling Bricolage by doing

sudo make db
sudo make db_grant

Answer “yes” when it asks you to drop the database. (Note: if there were any database changes, you’ll also need to run upgrade scripts in inst/upgrade/.) Then to get useful debugging output, do verbose testing with something like

sudo make devtest TEST_VERBOSE=1 > test.out 2>&1

All of the tests are run by the inst/runtests.pl script, which in turn tells “Test::Harness”harness to execute t/Bric/Test/Runner.pm. This file will find all the necessary test classes, load them, and then run their tests. inst/runtests.pl takes a number of arguments to simplify the running of scripts, including a list of test files or classes to run, so that you can just run the tests you need to run while you’re developing new tests. perldoc inst/runtests.pl for more information.

Be sure to use the testing base classes in your test classes. For non-database dependent tests (which are always in files named Test.pm, use Bric::Test::Base. For development tests, use Bric::Test::DevBase, which inherits from Bric::Test::Base. Be sure to read the documentation in these classes, as it will help you to write your tests more effectively. Pay special attention to the methods added to Bric::Test::DevBase, as they’re there to help you clean up any new records you’ve added to the database.

And finally, when you do provide patches to Bricolage, along with the new tests to test them, make sure that all existing tests pass as well. This means that you should always run make devtest on a fresh database build with your changes. Furthermore, if your patch involves changes to the database, you should provide the necessary upgrade script in inst/upgrade/, and also run the tests against a database that has been built from Bricolage sources untouched by your patch, and then upgraded by your upgrade script. This will help to ensure that your upgrade script modifies the database in the same way as your patch modifies the Bricolage SQL files.

And with that said, happy testing!