Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

include style guide for easyconfigs (REVIEW) #311

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/Changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Changelog for EasyBuild documentation

(for EasyBuild release notes, see :ref:`release_notes`)

* **release 20170227.01** (`Feb 27th 2017`): document 'code' :ref:`code_style_easyconfigs`
* **release 20170221.01** (`Feb 21st 2017`): add documentation on :ref:`contributing`
* **release 20170209.01** (`Feb 9th 2017`): add documentation on implementing easyblocks (see :ref:`implementing_easyblocks`)
* **release 20170203.01** (`Feb 3rd 2017`): update release notes for EasyBuild v3.1.0 (see :ref:`release_notes_eb310`)
Expand Down
305 changes: 303 additions & 2 deletions docs/Code_style.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,310 @@ The only (major) exception to PEP8 is our preference for longer line lengths: li

.. _PEP8: http://www.python.org/dev/peps/pep-0008

.. _code_style_easyconfigs:

Notes
~~~~~
Easyconfigs style guide
-----------------------

We maintain a strict 'code' style for easyconfig files too, which has proved
to be invaluable in making easyconfig files easy to grasp at a glance.

Major attention points include:

* :ref:`code_style_easyconfigs_max_line_length`
* :ref:`code_style_easyconfigs_whitespace`
* :ref:`code_style_easyconfigs_indentation`
* :ref:`code_style_easyconfigs_order_grouping`
* :ref:`code_style_easyconfigs_hardcoding`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hardcoding = templates_constants?

* :ref:`code_style_easyconfigs_templates_constants`
* :ref:`code_style_easyconfigs_string_quotes`
* :ref:`code_style_easyconfigs_file_names`

.. note:: You can check whether easyconfigs adhere to the easyconfig style rules using ``eb --check-style``.


.. _code_style_easyconfigs_max_line_length:

Maximum line length
~~~~~~~~~~~~~~~~~~~

**Each line must contain no more than 120 characters.**

If a parameter value is too long line wrapping should be used
or the value should be constructed differently.

For example, for a long ``description`` value, line wrapping can be used:

.. code:: python

description = """A long description with multiple lines,
that wraps around to the next line, and then continues on
to the line after that"""

Usually, a single leading space is used on continuation lines for descriptions
that are wrapped across multiple lines.

For a long value of ``configopts``, string concatenation using '``+=```' can be used.
Do make sure to include a space either and the end of all but the last partial
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

either at the end

value, or at the beginning of each partial value except the first one:

.. code:: python

configopts = "--first-option --second-option --third-option "
configopts += "--yet-another-option"

For a parameter value that is a long list, either line wrapping can be used
or each list element can be put on a separate line, see :ref:`code_style_easyconfigs_lists`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/code_style_easyconfigs_lists/code_style_easyconfigs_indentation/?



.. _code_style_easyconfigs_whitespace:

Whitespace
~~~~~~~~~~

Whitespace (i.e., spaces, tabs) is an integral part of Python syntax,
and hence very important.

In easyconfigs specifically, all **parameter definitions must be left-aligned**,
i.e., no whitespace to the left of the names of the parameters being defined
is allowed. Not honoring this rule will result in ``SyntaxError``'s.

On top of that, a couple of additional whitespace style rules must be taken into account:

* **no tab characters used for indentation**

* each tab must be replaced with *exactly 4 spaces*
* see also :ref:`code_style_easyconfigs_indentation`

* **no whitespace on blank lines**
* **no multiple blank lines in a row**
* **no trailing whitespace**, i.e., no extra spaces/tabs at the end of lines
* **a single space must be included before and after an assignment operator** '``=``'
* **a single space must be included (only) after commas** ``,`` **and colons** ``:`` (*not* before)
* **no spaces directly after** ``(``, ``[`` **or** ``{`, **nor before** ``)``, ``]`` **or** ``}`` characters

In addition, a single blank line must be used to separate groups of parameter definitions
(see :ref:`code_style_easyconfigs_order_grouping`) and to aid with readability.


.. _code_style_easyconfigs_indentation:

Indentation for list and dictionary values
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

**Indentation must be used for list and dictionary parameter values that
are spread across multiple lines.**

Each indentation level corresponds to exactly 4 spaces; *do not use
tab characters for indentation* (see :ref:`code_style_easyconfigs_whitespace`).

For long lists, you can either put each list element on a new line and
indent with (exactly) 4 spaces,
or simply break the list across multiple lines while aligning the first list element on
each line.

Which formatting style you should for lists use depends on the length of the individual
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should use for lists

list elements and the length of the entire list:

* short elements suggest simply breaking the list across multiple lines;
* long elements suggest one list element per line;
* long lists suggest avoiding a single element per line, to avoid consuming a lot of vertical space

In addition, a single list element per line allows for including comments for particular list elements.

With the above in mind it is difficult to prescribe strict rules for picking a formatting style for lists,
so you will need to pick one yourself (taking into account :ref:`code_style_easyconfigs_max_line_length`).

For dictionary values, it is custom to put each key-value pair on a separate line,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is customary?

and to indent each line using exactly 4 spaces.

For example:

.. code:: python

sources = [SOURCE_TAR_GZ]

# example of list value spread across multiple lines with one element per line
patches = [
'fix-compilation.patch', # patch to fix compilation problem
'backport-bugfix.patch', # patch to backport bug fix to this version
]

# example of list value spread across multiple lines by breaking the list
sanity_check_paths = {
'files': ['bin/example1', 'bin/example2', 'bin/example3', 'bin/example4',
'lib/libexample1.a', 'lib/libexample2.a'],
'dirs': ['example_directory'],
}


.. _code_style_easyconfigs_order_grouping:

Order & grouping of easyconfig parameters
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

**Parameter definitions must be ordered and organised in groups consistently across easyconfig files.**

Even though the order of the parameter definitions in easyconfig files (mostly) doesn't matter,
maintaining a consistent order across easyconfig files helps to make them easier to digest at a glance.

Related easyconfig parameters must grouped together, with a (single) blank included between groups of parameters.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

must be grouped?


.. note:: Order only matters when a particular parameter definition is (partially) defined in terms of another
parameter, for example when ``version`` is used to define one of the values in ``sources``.

This only applies to definitions that use the ``%`` operator rather than an equivalent template
like ``%(version)s``.

Parameter definitions in easyconfig files must be ordered/groups according to the following rules:

* if the ``easyblock`` parameter is defined it must be listed first, followed by a blank line;

* ``name`` and ``version`` must be next, in that order, grouped together and followed by a blank line;

* ``homepage`` and ``description`` are next, in that order, grouped together and followed by a blank line;

* ``toolchain`` must be next, optionally followed by ``toolchainopts`` (if defined), followed by a blank line;

* ``sources`` must be next (if defined), followed by a blank line;

* if ``source_urls`` is defined, it must be included right *before* ``sources`` (in the same group);
* if ``patches`` or ``checksums`` are defined, they must be included right *after* ``sources``, in that order (and in the same group);

* if ``builddependencies``, ``dependencies`` or ``osdependencies`` are defined they must be included next, grouped together;

* defined parameters that influence particular steps of the build and installation procedure must be included in order
and after the parameters mentioned above;

* i.e., ``(pre)configopts`` must be included before ``(pre)buildopts``, which must be included before ``(pre)installopts``, etc.;

* ``sanity_check_paths`` and ``sanity_check_commands`` must be included towards the end of the easyconfig file,
if they are defined;

* parameters influencing the contents of the generated module file (e.g., ``modextrapaths``, ``modextravars``, ...)
must be included *after* the ``sanity_check_*`` parameters, if they are defined

* ``moduleclass`` must be included as the last line

Example:

.. code:: python

# optional example header

easyblock = 'ConfigureMake'

name = 'example'
version = '1.2.3'

homepage = 'http://example.com'
description = "Example description"

toolchain = {'name': 'dummy', 'version': ''}
toolchainopts = {'pic': True}

source_urls = ['http://example.com']
sources = [SOURCE_TAR_GZ]
checksums = ['41150b03ec5b7f5a49390cc10eeb96d5']

configopts = '--with-foo'

prebuildopts = 'export COMPILER_FLAGS="$CFLAGS" && '
buildopts = 'CC="$CC"'

sanity_check_paths = {
'files': ['example'],
'dirs': [],
}

modextrapaths = {'PATH': ''}

moduleclass = ''


.. _code_style_easyconfigs_templates_constants:

Use of templates rather than hardcoding parameter values
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

**Hardcoding of parameter values in multiple places must be avoided if possible**,
the available easyconfig templates ``%(...)s`` must be used instead (see :ref:`avail_easyconfig_templates`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/avail_easyconfig_templates/easyconfig_param_templates/?


For example, rather than hardcoding the software version in both the ``version`` and ``sources``
parameter definitions, the ``%(version)s`` template value should be used instead:

.. code:: python

name = 'example'
version = '1.2.3'
...
sources = ['%(name)s-v%(version)s.tar.gz']

Commonly used templates include:

* ``%(namelower)s`` for the lower-case software name
* ``%(version)s`` for the full software version
* ``%(version_major)s``, ``%(version_minor)s``, ``%(version_major_minor)s`` for partial software versions
* ``SOURCE_TAR_GZ``, ``SOURCE_TGZ``, etc. for the ``sources`` parameter
* ``GNU_SOURCE``, ``PYPI_SOURCE``, ``SOURCEFORGE_SOURCE``, etc. for the ``source_urls`` parameter
* ``%(pyver)s`` and ``%(pyshortver)s`` for the (partial) Python version
* ``%(installdir)s`` for the software installation prefix
* ``SHLIB_EXT`` for the extension of shared libraries


.. _code_style_easyconfigs_string_quotes:

Single or double quotes for string values
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. note:: This is only a recommendation, it is not strictly applied in easyconfig files.

For string values, the following rules of thumb should be taken into account
with respect to the use of single or double quotes:

* use single quotes (``'...'``) for strings representing a single character or 'word' (i.e., a string with no spaces)
* use double quotes (``"..."``) for strings that include one or more spaces
* use triple-quoting (``"""..."""``) for multi-line strings

These guidelines can be ignored if there is a technical reason for doing so,
for example if double quotes *must* be used to ensure bash expansion of environment variables
(see ``buildopts`` in the example below).

For example:

.. code:: python

name = 'example'
version = '1.0'

homepage = 'http://example.com'
description = """A long description with multiple lines,
that wraps around to the next line"""

toolchain = {'name': 'foss', 'version': '2017a'}

sources = ['example-v%(version)s.tar.gz']

configopts = "--enable-stuff --with-more-stuff --disable-other-stuff"

buildopts = 'CC="$CC" CFLAGS="$CFLAGS"'

moduleclass = 'tools'


.. _code_style_easyconfigs_file_names:

Easyconfig file names
~~~~~~~~~~~~~~~~~~~~~

**Easyconfig filenames must follow the pattern** ``<name>-<version>[-<toolchain>][<versionsuffix>].eb``.

The ``toolchain`` part is omitted when the ``dummy`` toolchain is used; the ``versionsuffix`` part is omitted when the ``versionsuffix`` is empty.

This is strictly enforced because the 'robot' dependency resolution mechanism relies on easyconfig filenames, see also :ref:`robot_search_path`.

Links
-----

Style guides that go a step beyond PEP8:
* http://www.gramps-project.org/wiki/index.php?title=Programming_guidelines
Expand Down
2 changes: 2 additions & 0 deletions docs/Contributing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,8 @@ that mostly matches the established PEP8 coding style for Python (since
easyconfigs are written in Python syntax). However, also the grouping and
ordering of easyconfig parameters is a part of the 'code' style we maintain.

See :ref:`code_style_easyconfigs` for more information.


.. _contributing_review_process_review_pr:

Expand Down
2 changes: 1 addition & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
# The short X.Y version.
version = '3.1.0' # this is meant to reference the version of EasyBuild
# The full version, including alpha/beta/rc tags.
release = '20170221.01' # this is meant to reference the version of the documentation itself
release = '20170227.01' # this is meant to reference the version of the documentation itself

# There are two options for replacing |today|: either, you set today to some
# non-false value, then it is used:
Expand Down