Contributing Guide¶
First and foremost, thank you for wanting to contribute! It’s the only way open source works!
Before you dive into writing patches, here are some of the basics:
- Project page: http://launchpad.net/horizon
- Bug tracker: https://bugs.launchpad.net/horizon
- Source code: https://github.com/openstack/horizon
- Code review: https://review.openstack.org/#q,status:open+project:openstack/horizon,n,z
- Continuous integration: * Jenkins: https://jenkins.openstack.org * Zuul: http://status.openstack.org/zuul
- IRC Channel: #openstack-horizon on Freenode.
Making Contributions¶
Getting Started¶
We’ll start by assuming you’ve got a working checkout of the repository (if not then please see the Quickstart).
Second, you’ll need to take care of a couple administrative tasks:
- Create an account on Launchpad.
- Sign the OpenStack Contributor License Agreement and follow the associated instructions to verify your signature.
- Join the Horizon Developers team on Launchpad.
- Follow the instructions for setting up git-review in your development environment.
Whew! Got all that? Okay! You’re good to go.
Ways To Contribute¶
The easiest way to get started with Horizon’s code is to pick a bug on
Launchpad that interests you, and start working on that. Bugs tagged as
low-hanging-fruit
are a good place to start. Alternatively, if there’s an
OpenStack API feature you would like to see implemented in Horizon feel free
to try building it.
If those are too big, there are lots of great ways to get involved without plunging in head-first:
- Report bugs, triage new tickets, and review old tickets on the bug tracker.
- Propose ideas for improvements via Launchpad Blueprints, via the mailing list on the project page, or on IRC.
- Write documentation!
- Write unit tests for untested code!
- Help improve the User Experience Design or contribute to the Persona Working Group.
Choosing Issues To Work On¶
In general, if you want to write code, there are three cases for issues you might want to work on:
- Confirmed bugs
- Approved blueprints (features)
- New bugs you’ve discovered
If you have an idea for a new feature that isn’t in a blueprint yet, it’s a good idea to write the blueprint first, so you don’t end up writing a bunch of code that may not go in the direction the community wants.
For bugs, open the bug first, but if you can reproduce the bug reliably and identify its cause then it’s usually safe to start working on it. However, getting independent confirmation (and verifying that it’s not a duplicate) is always a good idea if you can be patient.
After You Write Your Patch¶
Once you’ve made your changes, there are a few things to do:
- Make sure the unit tests and linting tasks pass by running
tox
- Take a look at your patch in API profiler, i.e. how it impacts the performance. See Profiling Pages.
- Make sure your code is ready for translation: See Pseudo translation tool.
- Make sure your code is up-to-date with the latest master:
git pull --rebase
- Finally, run
git review
to upload your changes to Gerrit for review.
The Horizon core developers will be notified of the new review and will examine it in a timely fashion, either offering feedback or approving it to be merged. If the review is approved, it is sent to Jenkins to verify the unit tests pass and it can be merged cleanly. Once Jenkins approves it, the change will be merged to the master repository and it’s time to celebrate!
Profiling Pages¶
In Ocata release of Horizon a new “OpenStack Profiler” panel is introduced within a Developer dashboard. Once it is enabled and all prerequisites are set up, you can see what API calls Horizon actually makes when rendering a specific page. To re-render the page while profiling it, you’ll need to use “Profile” drop-down menu located left to the User menu in top right corner of the screen. In order to be able to use “Profile” menu the following steps need to be done:
#. Ensure that the Developer dashboard is enabled (copy _9001_developer.py file
from the openstack_dashboard/contrib/developer/enabled folder into the
openstack_dashboard/local/enabled folder if it is not already there).
#. Copy openstack_dashboard/local/local_settings.d/_9030_profiler_settings.py.example
file to openstack_dashboard/local/local_settings.d/_9030_profiler_settings.py
#. Copy openstack_dashboard/contrib/developer/enabled/_9030_profiler.py to
openstack_dashboard/local/enabled/_9030_profiler.py .
#. To support storing profiler data on server-side, MongoDB cluster needs
to be installed on Devstack host (default configuration), see Installing MongoDB.
Then, change net:bindIp: key to 0.0.0.0 inside /etc/mongod.conf and invoke
sudo service mongod restart
for the changes to have an effect.
#. Re-collect and re-compress static assets.
#. Re-start the production web-server in case you are serving Horizon from it.
#. The “Profile” drop-down menu should appear in the top-right corner, you are
ready to profile your pages!
Etiquette¶
The community’s guidelines for etiquette are fairly simple:
- Treat everyone respectfully and professionally.
- If a bug is “in progress” in the bug tracker, don’t start working on it without contacting the author. Try on IRC, or via the launchpad email contact link. If you don’t get a response after a reasonable time, then go ahead. Checking first avoids duplicate work and makes sure nobody’s toes get stepped on.
- If a blueprint is assigned, even if it hasn’t been started, be sure you contact the assignee before taking it on. These larger issues often have a history of discussion or specific implementation details that the assignee may be aware of that you are not.
- Please don’t re-open tickets closed by a core developer. If you disagree with the decision on the ticket, the appropriate solution is to take it up on IRC or the mailing list.
- Give credit where credit is due; if someone helps you substantially with a piece of code, it’s polite (though not required) to thank them in your commit message.
Code Style¶
As a project, Horizon adheres to code quality standards.
Python¶
We follow PEP8 for all our Python code, and use pep8.py
(available
via the shortcut tox -e pep8
) to validate that our code
meets proper Python style guidelines.
Django¶
Additionally, we follow Django’s style guide for templates, views, and other miscellany.
JavaScript¶
The following standards are divided into required and recommended sections. Our main goal in establishing these best practices is to have code that is reliable, readable, and maintainable.
Required¶
Reliable
- The code has to work on the stable and latest versions of Firefox, Chrome, Safari, and Opera web browsers, and on Microsoft Internet Explorer 11 and later.
- If you turned compression off during development via
COMPRESS_ENABLED = False
in local_settings.py, re-enable compression and test your code before submitting. - Use
===
as opposed to==
for equality checks. The==
will do a type cast before comparing, which can lead to unwanted results.
Note
If typecasting is desired, explicit casting is preferred to keep the meaning of your code clear.
Keep document reflows to a minimum. DOM manipulation is expensive, and can become a performance issue. If you are accessing the DOM, make sure that you are doing it in the most optimized way. One example is to build up a document fragment and then append the fragment to the DOM in one pass instead of doing multiple smaller DOM updates.
Use “strict”, enclosing each JavaScript file inside a self-executing function. The self-executing function keeps the strict scoped to the file, so its variables and methods are not exposed to other JavaScript files in the product.
Note
Using strict will throw exceptions for common coding errors, like accessing global vars, that normally are not flagged.
Example:
(function(){ 'use strict'; // code... })();
Use
forEach
|each
when looping whenever possible. AngularJS and jQuery both provide for each loops that provide both iteration and scope.AngularJS:
angular.forEach(objectToIterateOver, function(value, key) { // loop logic });
jQuery:
$.each(objectToIterateOver, function(key, value) { // loop logic });
Do not put variables or functions in the global namespace. There are several reasons why globals are bad, one being that all JavaScript included in an application runs in the same scope. The issue with that is if another script has the same method or variable names they overwrite each other.
Always put
var
in front of your variables. Not puttingvar
in front of a variable puts that variable into the global space, see above.Do not use
eval( )
. The eval (expression) evaluates the expression passed to it. This can open up your code to security vulnerabilities and other issues.Do not use ‘
with
object {code}’. Thewith
statement is used to access properties of an object. The issue withwith
is that its execution is not consistent, so by reading the statement in the code it is not always clear how it is being used.
Readable & Maintainable
Give meaningful names to methods and variables.
Avoid excessive nesting.
Avoid HTML and CSS in JS code. HTML and CSS belong in templates and stylesheets respectively. For example:
In our HTML files, we should focus on layout.
- Reduce the small/random
<script>
and<style>
elements in HTML. - Avoid in-lining styles into element in HTML. Use attributes and classes instead.
- Reduce the small/random
In our JS files, we should focus on logic rather than attempting to manipulate/style elements.
Avoid statements such as
element.css({property1,property2...})
they belong in a CSS class.Avoid statements such as
$("<div><span>abc</span></div>")
they belong in a HTML template file. Useshow
|hide
|clone
elements if dynamic content is required.Avoid using classes for detection purposes only, instead, defer to attributes. For example to find a div:
<div class="something"></div> $(".something").html("Don't find me this way!");
Is better found like:
<div data-something></div> $("div[data-something]").html("You found me correctly!");
Avoid commented-out code.
Avoid dead code.
Performance
Avoid creating instances of the same object repeatedly within the same scope. Instead, assign the object to a variable and re-use the existing object. For example:
$(document).on('click', function() { /* do something. */ }); $(document).on('mouseover', function() { /* do something. */ });
A better approach:
var $document = $(document); $document.on('click', function() { /* do something. */ }); $document.on('mouseover', function() { /* do something. */ });
In the first approach a jQuery object for
document
is created each time. The second approach creates only one jQuery object and reuses it. Each object needs to be created, uses memory, and needs to be garbage collected.
Recommended¶
Readable & Maintainable
Put a comment at the top of every file explaining what the purpose of this file is when the naming is not obvious. This guideline also applies to methods and variables.
Source-code formatting – (or “beautification”) is recommended but should be used with caution. Keep in mind that if you reformat an entire file that was not previously formatted the same way, it will mess up the diff during the code review. It is best to use a formatter when you are working on a new file by yourself, or with others who are using the same formatter. You can also choose to format a selected portion of a file only. Instructions for setting up ESLint for Eclipse, Sublime Text, Notepad++ and WebStorm/PyCharm are provided.
Use 2 spaces for code indentation.
Use
{ }
forif
,for
,while
statements, and don’t combine them on one line.// Do this //Not this // Not this if(x) { if(x) if(x) y =x; y=x; y=x; }
Use ESLint in your development environment.
AngularJS¶
Note
This section is intended as a quick intro to contributing with AngularJS. For more detailed information, check the AngularJS Topic Guide.
“John Papa Style Guide”¶
The John Papa Style Guide is the primary point of reference for Angular code style. This style guide has been endorsed by the AngularJS team:
"The most current and detailed Angular Style Guide is the
community-driven effort led by John Papa and Todd Motto."
- http://angularjs.blogspot.com/2014/02/an-angularjs-style-guide-and-best.html
The style guide is found at the below location:
https://github.com/johnpapa/angular-styleguide
When reviewing / writing, please refer to the sections of this guide. If an issue is encountered, note it with a comment and provide a link back to the specific issue. For example, code should use named functions. A review noting this should provide the following link in the comments:
https://github.com/johnpapa/angular-styleguide#style-y024
In addition to John Papa, the following guidelines are divided into required and recommended sections.
Required¶
- Scope is not the model (model is your JavaScript Objects). The scope
references the model. Use isolate scopes wherever possible.
- https://github.com/angular/angular.js/wiki/Understanding-Scopes
- Read-only in templates.
- Write-only in controllers.
- Since Django already uses
{{ }}
, use{$ $}
or{% verbatim %}
instead.
ESLint¶
ESLint is a great tool to be used during your code editing to improve JavaScript quality by checking your code against a configurable list of checks. Therefore, JavaScript developers should configure their editors to use ESLint to warn them of any such errors so they can be addressed. Since ESLint has a ton of configuration options to choose from, links are provided below to the options Horizon wants enforced along with the instructions for setting up ESLint for Eclipse, Sublime Text, Notepad++ and WebStorm/PyCharm.
Instructions for setting up ESLint: ESLint setup instructions
Note
ESLint is part of the automated unit tests performed by Jenkins. The automated test use the default configurations, which are less strict than the configurations we recommended to run in your local development environment.
CSS¶
Style guidelines for CSS are currently quite minimal. Do your best to make the code readable and well-organized. Two spaces are preferred for indentation so as to match both the JavaScript and HTML files.
JavaScript and CSS libraries using xstatic¶
We do not bundle third-party code in Horizon’s source tree. Instead, we package the required files as xstatic Python packages and add them as dependencies to Horizon.
To create a new xstatic package:
- Check if the library is already packaged as xstatic on PyPi, by searching for the library name. If it already is, go to step 5. If it is, but not in the right version, contact the original packager to have them update it.
- Package the library as an xstatic package by following the instructions in xstatic documentation. Install the xstatic-release script and follow the instructions that come with it.
- Create a new repository under OpenStack. Use “xstatic-core” and
“xstatic-ptl” groups for the ACLs. Make sure to include the
-pypi-wheel-upload
job in the project config. - Set up PyPi to allow OpenStack (the “openstackci” user) to publish your package.
- Add the new package to global-requirements.
To make a new release of the package, you need to:
- Ensure the version information in the xstatic/pkg/<package name>/__init__.py file is up to date, especially the BUILD.
- Push your updated package up for review in gerrit.
- Once the review is approved and the change merged, request a release by updating or creating the appropriate file for the xstatic package in the releases repository under deliverables/_independent. That will cause it to be automatically packaged and released to PyPi.
Warning
Note that once a package is released, you can not “un-release” it. You should never attempt to modify, delete or rename a released package without a lot of careful planning and feedback from all projects that use it.
For the purpose of fixing packaging mistakes, xstatic has the build number mechanism. Simply fix the error, increment the build number and release the newer package.
Integrating a new xstatic package into Horizon¶
Having done a release of an xstatic package:
- Look for the upper-constraints.txt edit related to the xstatic release that was just
performed. One will be created automatically by the release process in the
openstack/requirements
project with the topic new-release. You should -1 that patch until you are confident Horizon does not break (or you have generated a patch to fix Horizon for that release.) If no upper-constraints.txt patch is automatically generated, ensure the releases yaml file created in the releases repository has the “include-pypi-link: yes” setting. - Pull that patch down so you have the edited upper-constraints.txt file locally.
- Set the environment variable UPPER_CONSTRAINTS_FILE to the edited upper-constraints.txt file name and run tests or local development server through tox. This will pull in the precise version of the xstatic package that you need.
- Move on to releasing once you’re happy the Horizon changes are stable.
Releasing a new compatible version of Horizon to address issues in the new xstatic release:
- Continue to -1 the upper-constraints.txt patch above until this process is complete. A +1 from a Horizon developer will indicate to the requirements team that the upper-constraints.txt patch is OK to merge.
- When submitting your changes to Horizon to address issues around the new xstatic release, use a Depends-On: referencing the upper-constraints.txt review. This will cause the OpenStack testing infrastructure to pull in your updated xstatic package as well.
- Merge the upper-constraints.txt patch and the Horizon patch noting that Horizon’s gate may be broken in the interim between these steps, so try to minimise any delay there. With the Depends-On it’s actually safe to +W the Horizon patch, which will be held up until the related upper-constraints.txt patch merges.
HTML¶
Again, readability is paramount; however be conscientious of how the browser will handle whitespace when rendering the output. Two spaces is the preferred indentation style to match all front-end code.
Exception Handling¶
Avoid propogating direct exception messages thrown by OpenStack APIs to the UI.
It is a precaution against giving obscure or possibly sensitive data to a user.
These error messages from the API are also not translatable. Until there is a
standard error handling framework implemented by the services which presents
clean and translated messages, horizon catches all the exceptions thrown by the
API and normalizes them in horizon.exceptions.handle()
.
Documentation¶
Horizon’s documentation is written in reStructuredText (reST) and uses Sphinx for additional parsing and functionality, and should follow standard practices for writing reST. This includes:
- Flow paragraphs such that lines wrap at 80 characters or less.
- Use proper grammar, spelling, capitalization and punctuation at all times.
- Make use of Sphinx’s autodoc feature to document modules, classes and functions. This keeps the docs close to the source.
- Where possible, use Sphinx’s cross-reference syntax (e.g.
:class:`~horizon.foo.Bar
) when referring to other Horizon components. The better-linked our docs are, the easier they are to use.
Be sure to generate the documentation before submitting a patch for review. Unexpected warnings often appear when building the documentation, and slight reST syntax errors frequently cause links or cross-references not to work correctly.
Documentation is generated with Sphinx using the tox command. To create HTML docs and man pages:
$ tox -e docs
The results are in the doc/build/html and doc/build/man directories respectively.
Conventions¶
Simply by convention, we have a few rules about naming:
- The term “project” is used in place of Keystone’s “tenant” terminology in all user-facing text. The term “tenant” is still used in API code to make things more obvious for developers.
- The term “dashboard” refers to a top-level dashboard class, and “panel” to the sub-items within a dashboard. Referring to a panel as a dashboard is both confusing and incorrect.
Release Notes¶
Release notes for a patch should be included in the patch with the associated changes whenever possible. This allow for simpler tracking. It also enables a single cherry pick to be done if the change is backported to a previous release. In some cases, such as a feature that is provided via multiple patches, release notes can be done in a follow-on review.
If the following applies to the patch, a release note is required:
- The deployer needs to take an action when upgrading
- A new feature is implemented
- Function was removed (hopefully it was deprecated)
- Current behavior is changed
- A new config option is added that the deployer should consider changing from the default
- A security bug is fixed
A release note is suggested if a long-standing or important bug is fixed. Otherwise, a release note is not required.
Horizon uses reno to generate release notes. Please read the docs for details. In summary, use
$ tox -e venv -- reno new <bug-,bp-,whatever>
Then edit the sample file that was created and push it with your change.
To see the results:
$ git commit # Commit the change because reno scans git log.
$ tox -e releasenotes
Then look at the generated release notes files in releasenotes/build/html in your favorite browser.