Latest revision as of 23:32, 28 March 2017

Status: This page is work in progress and has very questionable content :)

MusicBrainz Server Coding Style

Perl Modules

Use spaces, not tabs.
4 spaces for indentation
Opening curly braces go on the same line as their control statements.
Name references to the current object $self, not $this.
In Catalyst controllers, the Catalyst context should be stored in $c.
Use words separated by underscores for function/method names - never use uppercase characters
Everything mentioned in http://perldoc.perl.org/perlstyle.html -- unless specifically overridden by the rules above.

Note

The $c->model function looks in /lib/MusicBrainz/Server/Data/ to load the class whose name it is given. The /lib/MusicBrainz/Server/Model/ directory is not used for this, oddly enough.

Perl Module Template and Documentation Guidelines

package MusicBrainz::Server::*;

use strict;
use warnings;

use base 'Whatever'; # If needed

=head1 NAME

MusicBrainz::Server::* - one line description

=head1 DESCRIPTION

Larger description of the module - a few paragraphs.

=head1 ATTRIBUTES

Public attributes (variables with get/set routines)

=head1 METHODS

=head2 method_name $parameters, \%otherParams, \@moreParams, ?$optional

There doesn't seem to be any real standard for documentating sub-routines, so try
to stick as close to the above format. Include all parameters that will be used, and
prefix with a \ if they should be passed by reference. You can also flag parameters
as optional with a ? (it may be worth specifying the optional value too).

When documenting a subroutine, specify what it does, not how it does it.

=cut

1;

=head1 COPYRIGHT AND LICENSE

Copyright (C) YEAR(s) MetaBrainz Foundation

This file is part of MusicBrainz, the open internet music database,
and is licensed under the GPL version 2, or (at your option) any
later version: http://www.gnu.org/licenses/gpl-2.0.txt

=cut

Use objects

Various bits of musicbrainz server code currently pass around hashrefs of data which would be better represented with objects. When making changes to existing code, try to move any hashref using code to use object instances instead (ref: irc).

Suggested Workflow

Topical branches

You should create a new branch off the latest trunk for any work you are doing. The idea is to isolate all changes required for one feature into a set of commits - so reviewers and integrators can easily check your code and merge it. If you use Git, it's as simple as just doing:

git checkout -b my-feature master # Assuming master holds the contents of trunk

Submitting For Review

We now require all patches to be reviewed by at least one other coder and approved, before the code can be integrated into trunk. To submit code for review, you should squash all your patches into a single patch, and submit this to mb-devel. Then, work with developers, and eventually someone will merge the code.

Authentication

To require authenticated users, just forward to the /user/login action first. This will present users with a login form, and handle redirects correctly for you.

sub destroy_world : Local {
    my ($self, $c) = @_;
    $c->forward('/user/login');
}

Templates

When working with templates, in general try and be as concise as possible. If you find yourself doing a lot of logic, you should seek to abstract this out into a template component (more on these below) - or move the logic to the controller action; filing a bug on the bug tracker if you don't want to do this yourself.

Forms

I've tried to make working with forms as easy as possible, and there are 2 options. A generic form has labels on the left and inputs on the right (ie, login, artist edit, pretty much all of our forms). Custom forms impose no such restrictions, and allow you to layout the form however you want.

When working with any form, use the forms/form.tt wrapper. This wrapper sets up the <form> tags for you, along with providing the user information regarding any general errors and which fields are mandatory. By default the form is setup as a generic form, however if you pass the custom=1 option to the wrapper, no formatting will be done for you.

Once you have opened a form wrapper you are ready to begin your form. To add a widget to the form with a label (textbox, combo-box, etc) - you should INCLUDE the forms/widget.tt component. forms/widget.tt requires the following options:

widget: The field in the form that you wish to display. You will often fetch this by using form.field('field-name') .
label: The label of this field.

You should then finish your form with forms/submit.tt , providing the text for the button with the label parameter.

Below is a simple example form. For an example of a more complicated custom form, see the user preferences form

[% WRAPPER "forms/form.tt" %]
  [% INCLUDE "forms/widget.tt" widget=form.fields('name') label="Full Name:" %]
  [% INCLUDE "forms/submit.tt" label="Save Name" %]
[% END %]

User Preferences

You can access the current users preferences via the $c->user->preferences variable ( c.user.preferences in templates). For example:

[% IF c.user.preferences.get('preference') %]

where preference is whatever the preference's variable is in the perl files below.

To add a preference setting, add it to:

/root/user/preferences.tt
/lib/UserPreference.pm
/lib/MusicBrainz/Server/Form/User/Preferences.pm

How to add Guess Case and the Edit Suite to any template

Step 1

Template before:

[% extra_js = "switch.js switchcontrols.js " %]
[% WRAPPER 'layout.tt' title=l('Add Release') %]

Template after:

[% extra_js = "switch.js switchcontrols.js " %]
[% guessCase=1 undoRedo=1 searchReplace=1 trackParser=1 styleGuidelines=1 %]
[% PROCESS 'editsuite/suitereqs.tt' %]
[% WRAPPER 'layout.tt' title=l('Add Release') %]

Note that this order is important. You can add other lines in-between, if you would like, but the two Edit Suite lines must be after the extra_js line, and before the layout.tt line.

In the above options, the (implied) SET line determines which modules you want to have loaded within that form - leave any out, and the HTML and JS won't even be inicluded; == smaller page to load. (ie, no need for TP on the "add artist" form)

Example:

[% guessCase=1 undoRedo=1 searchReplace=1 styleGuidelines=1 %]

Will load those 4 modules, but not the track parser.

The Edit Suite Settings panel is always available, if the user has it turned on in User Preferences. It does not need to be turned on in the above manner within the template code.

Keep in mind that if you want Guess Case on a form, even if you leave out every other module, you still need to include the Guess Case module:

[% guessCase=1 %]

Step 2

Put

[% INCLUDE 'editsuite/suiteloader.tt' %]

whereever you want the Edit Suite to be inserted into the form.

Step 3

Now, add hooks to the form fields so the various Edit Suite modules know what fields to work on.

All you need to do in your form is add a class to the fields. There are three field types: Artist, Text, and Time, plus one subsidiary type.

For Track Artist or Release Artist, add class es-artist to the INCLUDE for the input field. For Track Title, Release Title, or Label Name, add class es-text to the INCLUDE for the input field. For Track Time, add class es-time to the INCLUDE for the input field. For Label names and other fields that should be guess cased according to standard guess case rules for track titles, you can use es-text, except in one case mentioned below. You can also use es-label. es-text fields will get a button, es-label

@@ Line 6: / Line 6: @@
 ==Perl Modules==
-* Use tabs, not spaces.
+* Use spaces, not tabs.
+* 4 spaces for indentation
+* Opening curly braces go on the same line as their control statements.
 * Name references to the current object <code><nowiki>$self</nowiki></code>, not <code><nowiki>$this</nowiki></code>.
 * In Catalyst controllers, the Catalyst context should be stored in <code><nowiki>$c</nowiki></code>.
-* Use words separated by underscores for function/method names. The server currently uses CamelCase, but I'd propose to change this to match [http://perldoc.perl.org/perlstyle.html perlstyle] and most existing Perl modules.
+* Use words separated by underscores for function/method names - never use uppercase characters
+* Everything mentioned in [http://perldoc.perl.org/perlstyle.html http://perldoc.perl.org/perlstyle.html] -- unless specifically overridden by the rules above.
-==Controllers==
+== Note ==
+* The <code>$c->model</code> function looks in <code>/lib/MusicBrainz/Server/Data/</code> to load the class whose name it is given. The <code>/lib/MusicBrainz/Server/Model/</code> directory is not used for this, oddly enough.
+==Perl Module Template and Documentation Guidelines==
-===Authentication===
+<pre>package MusicBrainz::Server::*;
-If an action in your controller requires the user to be logged in, you should add this action to the list of private pages in <code><nowiki> musicbrainz.yml </nowiki></code> (located in the root directory of your source code) - following the current format. The path like syntax can be found by looking at the controller listing in the Catalyst development server, but in general takes the form <code><nowiki> CONTROLLER/ACTION </nowiki></code> (so <code><nowiki> MusicBrainz::Server::Controller::Foo::bar </nowiki></code> would be entered as <code><nowiki> foo/bar </nowiki></code>).
+use strict;
-By taking this approach, you will not need to handle redirection. If you need more fine grained control (ie, you do not know whether the user should be logged in until you have data), here is a snippet which you can use to  seamlessly implement login redirection:
+use warnings;
+use base 'Whatever'; # If needed
-<pre>if (!$c->user_exists)
-{
+=head1 NAME
-    $c->flash->{login_redirect} = $c->uri_for($c->action, $c->req->args);
-    $c->response->redirect($c->uri_for('/user/login'));
+MusicBrainz::Server::* - one line description
-    $c->detach
+=head1 DESCRIPTION
+Larger description of the module - a few paragraphs.
+=head1 ATTRIBUTES
+Public attributes (variables with get/set routines)
+=head1 METHODS
+=head2 method_name $parameters, \%otherParams, \@moreParams, ?$optional
+There doesn't seem to be any real standard for documentating sub-routines, so try
+to stick as close to the above format. Include all parameters that will be used, and
+prefix with a \ if they should be passed by reference. You can also flag parameters
+as optional with a ? (it may be worth specifying the optional value too).
+When documenting a subroutine, specify what it does, not how it does it.
+=cut
+;
+=head1 COPYRIGHT AND LICENSE
+Copyright (C) YEAR(s) MetaBrainz Foundation
+This file is part of MusicBrainz, the open internet music database,
+and is licensed under the GPL version 2, or (at your option) any
+later version: http://www.gnu.org/licenses/gpl-2.0.txt
+=cut
+</pre>
+==Use objects==
+Various bits of musicbrainz server code currently pass around hashrefs of data which would be better represented with objects.  When making changes to existing code, try to move any hashref using code to use object instances instead (ref: [http://chatlogs.musicbrainz.org/musicbrainz-devel/2012/2012-01/2012-01-31.html#T16-55-02-124722 irc]).
+=Suggested Workflow=
+==Topical branches==
+You should create a new branch off the latest trunk for any work you are doing. The idea is to isolate all changes required for one feature into a set of commits - so reviewers and integrators can easily check your code and merge it. If you use Git, it's as simple as just doing:
+<pre>git checkout -b my-feature master # Assuming master holds the contents of trunk
+</pre>
+==Submitting For Review==
+We now require all patches to be reviewed by at least one other coder and approved, before the code can be integrated into trunk. To submit code for review, you should squash all your patches into a single patch, and submit this to mb-devel. Then, work with developers, and eventually someone will merge the code.
+==Authentication==
+To require authenticated users, just forward to the <code><nowiki>/user/login</nowiki></code> action first. This will present users with a login form, and handle redirects correctly for you.
+<pre>sub destroy_world : Local {
+    my ($self, $c) = @_;
+    $c->forward('/user/login');
 }
 </pre>
-==TT2 Templates==
+=Templates=
 When working with templates, in general try and be as concise as possible. If you find yourself doing a lot of logic, you should seek to abstract this out into a template component (more on these below) - or move the logic to the controller action; filing a bug on the bug tracker if you don't want to do this yourself.
-===Forms===
+=Forms=
-I've tried to make working with forms as easy as possible, and there are 2 options. I have noticed we have 2 types of forms, which I'll call generic and custom. A generic form has labels on the left and inputs on the right (ie, login, artist edit, pretty much all of our forms). Custom forms impose no such restrictions, and allow you to layout the form however you want.
+I've tried to make working with forms as easy as possible, and there are 2 options. A generic form has labels on the left and inputs on the right (ie, login, artist edit, pretty much all of our forms). Custom forms impose no such restrictions, and allow you to layout the form however you want.
 When working with any form, use the <code><nowiki> forms/form.tt </nowiki></code> wrapper. This wrapper sets up the <code><nowiki><form></nowiki></code> tags for you, along with providing the user information regarding any general errors and which fields are mandatory. By default the form is setup as a generic form, however if you pass the <code><nowiki>custom=1</nowiki></code> option to the wrapper, no formatting will be done for you.
@@ Line 49: / Line 113: @@
 </pre>
+=User Preferences=
-==CSS==
+You can access the current users preferences via the <code><nowiki> $c->user->preferences </nowiki></code> variable (<code><nowiki> c.user.preferences </nowiki></code> in templates). For example:
-* TODO
+<pre>[% IF c.user.preferences.get('preference') %]
-==JavaScript==
+</pre>
+where preference is whatever the preference's variable is in the perl files below.
-===How to add Guess Case and the Edit Suite to any template:===
+To add a preference setting, add it to:
-====Step 1====
+<ul><li style="list-style-type:none">/root/user/preferences.tt<br/> /lib/UserPreference.pm<br/> /lib/MusicBrainz/Server/Form/User/Preferences.pm
+</ul>
+=How to add Guess Case and the Edit Suite to any template=
+==Step 1==
 Template before: <pre>[% extra_js = "switch.js switchcontrols.js " %]
@@ Line 85: / Line 156: @@
 </pre>
-====Step 2====
+==Step 2==
 Put <pre>[% INCLUDE 'editsuite/suiteloader.tt' %]
@@ Line 92: / Line 163: @@
 whereever you want the Edit Suite to be inserted into the form.
-====Step 3====
+==Step 3==
 Now, add hooks to the form fields so the various Edit Suite modules know what fields to work on.
-All you need to do in your form is add a class to the fields.  There are three field types:  Artist, Title, and Time.
+All you need to do in your form is add a class to the fields.  There are three field types:  Artist, Text, and Time, plus one subsidiary type.
-For Track Artist or Release Artist, add class es-artist to the INCLUDE for the input field. For Track Title or Release Title, add class es-title to the INCLUDE for the input field. For Track Time, add class es-time to the INCLUDE for the input field.
-If you want to have a Guess All button on the form, add es=1 to the INCLUDE for the form.
-====Caution on Step 3====
-For each of es-artist, es-title, and es-time, you *can* leave out one or more of the three.  You can have the add artist form with just class es-artist and no es-title or es-time.
-However, when you have '''more than one of these fields in your form''', such as on the Add Release form, make sure that each is assigned to each field, ie, don't give class es-artist to the release artist and all track artists, but give class es-title only to the track titles, and not the release artist.  Otherwise, you will break track parser functionality.
-====Step 4====
-There is no spoon, nor is there a step 4.  It really is that easy now to add the Edit Suite and Guess Case functionality.  :)
-----
+For Track Artist or Release Artist, add class es-artist to the INCLUDE for the input field. For Track Title, Release Title, or Label Name, add class es-text to the INCLUDE for the input field. For Track Time, add class es-time to the INCLUDE for the input field. For Label names and other fields that should be guess cased according to standard guess case rules for track titles, you can use es-text, except in one case mentioned below.  You can also use es-label.  es-text fields will get a button, es-label
- [[Server Development|ServerDevelopment]]
-[[Category:To Be Reviewed]]
+[[Category:To Be Reviewed]] [[Category:Development]]

Development/Server Coding Style: Difference between revisions

Latest revision as of 23:32, 28 March 2017

Contents