2b7b8ac93c
This commit aligns module names with their directory structure. This is one step in a larger effort to restructure the Angular source. See https://review.openstack.org/#/c/176152/ for the full set of planned changes. Change-Id: I3f48ebaef23d6249983f33b1a4710cc1fe3ef789 Partial-Bug: #1454880
567 lines
22 KiB
ReStructuredText
567 lines
22 KiB
ReStructuredText
==================
|
||
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 :doc:`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. 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`_.
|
||
|
||
.. _`bug tracker`: https://bugs.launchpad.net/horizon
|
||
.. _`Launchpad Blueprints`: https://blueprints.launchpad.net/horizon
|
||
.. _`User Experience Design`: https://wiki.openstack.org/wiki/UX#Getting_Started
|
||
.. _`Persona Working Group`: https://wiki.openstack.org/wiki/Personas
|
||
|
||
|
||
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 pass: ``./run_tests.sh``
|
||
* Make sure PEP8 is clean: ``./run_tests.sh --pep8``
|
||
* Make sure your code is ready for translation: ``./run_tests.sh --pseudo de`` See the Translatability section below for details.
|
||
* 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!
|
||
|
||
.. _`OpenStack Contributor License Agreement`: http://wiki.openstack.org/CLA
|
||
.. _`OpenStack Contributors`: https://launchpad.net/~openstack-cla
|
||
.. _`Horizon Developers`: https://launchpad.net/~horizon
|
||
.. _`instructions for setting up git-review`: http://docs.openstack.org/infra/manual/developers.html#development-workflow
|
||
|
||
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.
|
||
|
||
Translatability
|
||
===============
|
||
Horizon gets translated into multiple languages. The pseudo translation tool
|
||
can be used to verify that code is ready to be translated. The pseudo tool
|
||
replaces a language's translation with a complete, fake translation. Then
|
||
you can verify that your code properly displays fake translations to validate
|
||
that your code is ready for translation.
|
||
|
||
Running the pseudo translation tool
|
||
-----------------------------------
|
||
|
||
#. Make sure your English po file is up to date: ``./run_tests.sh --makemessages``
|
||
#. Run the pseudo tool to create pseudo translations. For example, to replace the German translation with a pseudo translation: ``./run_tests.sh --pseudo de``
|
||
#. Compile the catalog: ``./run_tests.sh --compilemessages``
|
||
#. Run your development server.
|
||
#. Log in and change to the language you pseudo translated.
|
||
|
||
It should look weird. More specifically, the translatable segments are going
|
||
to start and end with a bracket and they are going to have some added
|
||
characters. For example, "Log In" will become "[~Log In~您好яшçあ]"
|
||
This is useful because you can inspect for the following, and consider if your
|
||
code is working like it should:
|
||
|
||
* If you see a string in English it's not translatable. Should it be?
|
||
* If you see brackets next to each other that might be concatenation. Concatenation
|
||
can make quality translations difficult or impossible. See
|
||
https://wiki.openstack.org/wiki/I18n/TranslatableStrings#Use_string_formating_variables.2C_never_perform_string_concatenation
|
||
for additional information.
|
||
* If there is unexpected wrapping/truncation there might not be enough
|
||
space for translations.
|
||
* If you see a string in the proper translated language, it comes from an
|
||
external source. (That's not bad, just sometimes useful to know)
|
||
* If you get new crashes, there is probably a bug.
|
||
|
||
Don't forget to cleanup any pseudo translated po files. Those don't get merged!
|
||
|
||
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 ``./run_tests.sh --pep8``) to validate that our code
|
||
meets proper Python style guidelines.
|
||
|
||
.. _PEP8: http://www.python.org/dev/peps/pep-0008/
|
||
|
||
Django
|
||
------
|
||
|
||
Additionally, we follow `Django's style guide`_ for templates, views, and
|
||
other miscellany.
|
||
|
||
.. _Django's style guide: https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/
|
||
|
||
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 9 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:
|
||
|
||
.. code ::
|
||
|
||
(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:
|
||
|
||
.. code ::
|
||
|
||
angular.forEach(objectToIterateOver, function(value, key) {
|
||
// loop logic
|
||
});
|
||
|
||
jQuery:
|
||
|
||
.. code ::
|
||
|
||
$.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 putting ``var`` 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}'. The ``with`` statement is used to access
|
||
properties of an object. The issue with ``with`` 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.
|
||
|
||
1. Reduce the small/random ``<script>`` and ``<style>`` elements in HTML.
|
||
|
||
2. Avoid in-lining styles into element in HTML. Use attributes and
|
||
classes instead.
|
||
* In our JS files, we should focus on logic rather than attempting to
|
||
manipulate/style elements.
|
||
|
||
1. Avoid statements such as ``element.css({property1,property2...})`` they
|
||
belong in a CSS class.
|
||
|
||
2. Avoid statements such as ``$("<div><span>abc</span></div>")`` they
|
||
belong in a HTML template file. Use ``show`` | ``hide`` | ``clone``
|
||
elements if dynamic content is required.
|
||
|
||
3. Avoid using classes for detection purposes only, instead, defer to
|
||
attributes. For example to find a div:
|
||
.. code ::
|
||
|
||
<div class="something"></div>
|
||
$(".something").html("Don't find me this way!");
|
||
|
||
Is better found like:
|
||
|
||
.. code ::
|
||
|
||
<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:
|
||
|
||
.. code ::
|
||
|
||
$(document).on('click', function() { /* do something. */ });
|
||
$(document).on('mouseover', function() { /* do something. */ });
|
||
|
||
A better approach:
|
||
|
||
.. code ::
|
||
|
||
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 JSHint for Eclipse, Sublime Text, Notepad++ and WebStorm/PyCharm are
|
||
provided_.
|
||
* Use 2 spaces for code indentation.
|
||
* Use ``{ }`` for ``if``, ``for``, ``while`` statements, and don't combine them
|
||
on one line.
|
||
|
||
.. code ::
|
||
|
||
// Do this //Not this // Not this
|
||
if(x) { if(x) if(x) y =x;
|
||
y=x; y=x;
|
||
}
|
||
* Use JSHint in your development environment.
|
||
|
||
|
||
AngularJS
|
||
---------
|
||
|
||
"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
|
||
~~~~~~~~
|
||
|
||
* Organization: Define your Angular app under the root Angular folder (such
|
||
as ``horizon/static/horizon/js/angular/hz.table.js``).
|
||
|
||
* Controllers and Services should not contain DOM references. Directives
|
||
should.
|
||
* Services are singletons and contain logic independent of view.
|
||
* 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.
|
||
* For localization in Angular files, use the Angular service horizon.framework.util.i18n.gettext.
|
||
For regular Javascript files, use either ``gettext`` or ``ngettext``.
|
||
Only those two methods are recognized by our tools and will be included in
|
||
the .po file after running ``./run_tests --makemessages``.
|
||
|
||
.. code ::
|
||
|
||
// Angular
|
||
angular.module('horizon.framework.widgets.modal', ['ui.bootstrap', 'horizon.framework.util.i18n'])
|
||
.factory('simpleModalService', ['$modal', 'basePath', 'horizon.framework.util.i18n.gettext',
|
||
function($modal, path, gettext) { gettext('translatable text'); };
|
||
|
||
// recognized
|
||
gettext("translatable text");
|
||
ngettext("translatable text");
|
||
|
||
// not recognized
|
||
var _ = gettext;
|
||
_('translatable text');
|
||
|
||
$window.gettext('translatable text');
|
||
|
||
Recommended
|
||
~~~~~~~~~~~
|
||
|
||
* Put "Ctrl" on the end of a controller file name.
|
||
* Don't use variables like "app" that are at the highest level in the file,
|
||
when Angular gives an alternative. For example use function chaining:
|
||
|
||
.. code ::
|
||
|
||
angular.module('my_module')
|
||
.controller('my_controller', ['$scope', function($scope) {
|
||
// controller code
|
||
}]).service('my_service', ['$scope', function($scope) {
|
||
// service code
|
||
}]);
|
||
|
||
|
||
JSHint
|
||
------
|
||
JSHint 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 JSHint
|
||
to warn them of any such errors so they can be addressed. Since JSHint 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
|
||
JSHint for Eclipse, Sublime Text, Notepad++ and WebStorm/PyCharm.
|
||
|
||
JSHint configuration file: `.jshintrc`_
|
||
|
||
Instructions for setting up JSHint: `JSHint setup instructions`_
|
||
|
||
.. Note ::
|
||
JSHint 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.
|
||
|
||
.. _.jshintrc: https://wiki.openstack.org/wiki/Horizon/Javascript/EditorConfig/Settings#.jshintrc
|
||
.. _JSHint setup instructions: https://wiki.openstack.org/wiki/Horizon/Javascript/EditorConfig
|
||
.. _provided: https://wiki.openstack.org/wiki/Horizon/Javascript/EditorConfig
|
||
|
||
|
||
|
||
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
|
||
----------------------------
|
||
|
||
We do not bundle the third-party code within Horizon's source tree anymore, any
|
||
code that is still there is just left over and will be cleaned up and packaged
|
||
properly eventually. What we do instead, is packaging the required files as
|
||
XStatic Python packages and adding them as dependencies to Horizon. In
|
||
particular, when you need to add a new third-party JavaScript or CSS library to
|
||
Horizon, follow those steps:
|
||
|
||
1. 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.
|
||
2. Package the library as an Xstatic package by following the instructions in
|
||
Xstatic documentation_.
|
||
3. `Create a new repository on StackForge`_. Use "xstatic-core" and
|
||
"xstatic-ptl" groups for the ACLs. Make sure to include the
|
||
``publish-to-pypi`` job.
|
||
4. `Setup PyPi`_ to allow OpenStack to publish your package.
|
||
5. `Tag your release`_. That will cause it to be automatically packaged and
|
||
released to PyPi.
|
||
6. Add the package to global-requirements_. Make sure to mention the license.
|
||
7. Add the package to Horizon's ``requirements.txt`` file, to its
|
||
``settings.py``, and to the ``_scripts.html`` or ``_stylesheets.html``
|
||
templates. Make sure to keep the order alphabetic.
|
||
|
||
.. _documentation: http://xstatic.rtfd.org/en/latest/packaging.html
|
||
.. _`Create a new repository on StackForge`: http://docs.openstack.org/infra/manual/creators.html
|
||
.. _global-requirements: https://github.com/openstack/requirements/blob/master/global-requirements.txt
|
||
.. _`Tag your release`: http://docs.openstack.org/infra/manual/drivers.html#tagging-a-release
|
||
.. _`Setup PyPi`: http://docs.openstack.org/infra/manual/creators.html#give-openstack-permission-to-publish-releases
|
||
|
||
|
||
.. warning::
|
||
|
||
Note that once a package is released, you can not "unrealease" 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.
|
||
|
||
|
||
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.
|
||
|
||
Documentation
|
||
-------------
|
||
|
||
Horizon's documentation is written in reStructuredText 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.
|
||
|
||
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.
|