contributing.rst 5.35 KB
Newer Older
Paul McCarthy's avatar
Paul McCarthy committed
1
2
Contributing to ``fslpy``
=========================
3
4


Paul McCarthy's avatar
Paul McCarthy committed
5
*This document is a work in progress*
6
7


Paul McCarthy's avatar
Paul McCarthy committed
8
9
Development model
-----------------
10
11


Paul McCarthy's avatar
Paul McCarthy committed
12
13
- The master branch should always be stable and ready to release. All
  development occurs on the master branch.
14

Paul McCarthy's avatar
Paul McCarthy committed
15
16
17
- All changes to the master branch occur via merge requests. Individual
  developers are free to choose their own development workflow in their own
  repositories.
Paul McCarthy's avatar
Paul McCarthy committed
18

Paul McCarthy's avatar
Paul McCarthy committed
19
- Merge requests will not be accepted unless:
20

Paul McCarthy's avatar
Paul McCarthy committed
21
22
23
24
 - All existing tests pass (or have been updated as needed).
 - New tests have been written to cover newly added features.
 - Code coverage is as close to 100% as possible.
 - Coding conventions are adhered to (unless there is good reason not to).
Paul McCarthy's avatar
Paul McCarthy committed
25

Paul McCarthy's avatar
Paul McCarthy committed
26

27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
Commit messages
---------------


To aid readability, all commit messages should be prefixed with one or more of
the following labels (this convention has been inherited from `nibabel
<https://github.com/nipy/nibabel>`_):

  * *BF* : bug fix
  * *RF* : refactoring
  * *NF* : new feature
  * *BW* : addresses backward-compatibility
  * *OPT* : optimization
  * *BK* : breaks something and/or tests fail
  * *PL* : making pylint happier
  * *DOC*: for all kinds of documentation related commits
  * *TEST*: for adding or changing tests
  * *MAINT*: for administrative/maintenance changes
Paul McCarthy's avatar
Paul McCarthy committed
45
  * *CI*: for continuous-integration changes
46
47


Paul McCarthy's avatar
Paul McCarthy committed
48
49
50
51
Version number
--------------


52
The ``fslpy`` version number roughly follows `semantic versioning
Paul McCarthy's avatar
Paul McCarthy committed
53
54
<http://semver.org/>`_ rules, so that dependant projects are able to perform
compatibility testing.  The full version number string consists of three
55
numbers::
Paul McCarthy's avatar
Paul McCarthy committed
56
57
58

    major.minor.patch

Paul McCarthy's avatar
Paul McCarthy committed
59
60
- The ``patch`` number is incremented on bugfixes and minor
  (backwards-compatible) changes.
61

Paul McCarthy's avatar
Paul McCarthy committed
62
63
- The ``minor`` number is incremented on feature additions and/or
  backwards-compatible changes.
Paul McCarthy's avatar
Paul McCarthy committed
64

Paul McCarthy's avatar
Paul McCarthy committed
65
66
- The ``major`` number is incremented on major feature additions, and
  backwards-incompatible changes.
Paul McCarthy's avatar
Paul McCarthy committed
67
68


Paul McCarthy's avatar
Paul McCarthy committed
69
70
71
72
73
74
The version number in the ``master`` branch should be of the form
``major.minor.patch.dev``, to indicate that any releases made from this branch
are development releases (although development releases are not part of the
release model).


75
76
77
78
79
80
Releases
--------


A separate branch is created for each **minor** release. The name of the
branch is ``v[major.minor]``, where ``[major.minor]`` is the first two
Paul McCarthy's avatar
Paul McCarthy committed
81
components of the release version number (see above). For example, the branch
82
83
84
name for minor release ``1.0`` would be ``v1.0``.


Paul McCarthy's avatar
Paul McCarthy committed
85
Patches and bugfixes may be added to these release branches as ``patch``
Paul McCarthy's avatar
Paul McCarthy committed
86
87
88
releases.  These changes should be made on the master branch like any other
change (i.e. via merge requests), and then cherry-picked onto the relevant
release branch(es).
89
90


Paul McCarthy's avatar
Paul McCarthy committed
91
92
Every release commit is also tagged with its full version number.  For
example, the first release off the ``v1.0`` branch would be tagged with
Paul McCarthy's avatar
Paul McCarthy committed
93
``1.0.0``.  Patch releases to the ``v1.0`` branch would be tagged with
Paul McCarthy's avatar
Paul McCarthy committed
94
``1.0.1``, ``1.0.2``, etc.
95
96


Paul McCarthy's avatar
Paul McCarthy committed
97
98
99
100
Testing
-------


Paul McCarthy's avatar
Paul McCarthy committed
101
Unit and integration tests are currently run with ``py.test`` and
Paul McCarthy's avatar
Paul McCarthy committed
102
``coverage``.
Paul McCarthy's avatar
Paul McCarthy committed
103

Paul McCarthy's avatar
Paul McCarthy committed
104
- Aim for 100% code coverage.
Paul McCarthy's avatar
Paul McCarthy committed
105
- Tests must pass on python 2.7, 3.4, 3.5, and 3.6
Paul McCarthy's avatar
Paul McCarthy committed
106
107
108
109
110
111


Coding conventions
------------------


Paul McCarthy's avatar
Paul McCarthy committed
112
113
114
115
116
117
- Clean, readable code is good
- White space and visual alignment is good (where it helps to make the code
  more readable)
- Clear and accurate documentation is good
- Document all modules, functions, classes, and methods using
  `ReStructuredText <http://www.sphinx-doc.org/en/stable/rest.html>`_.
Paul McCarthy's avatar
Paul McCarthy committed
118
119


Paul McCarthy's avatar
Paul McCarthy committed
120
Configure your text editor to use:
Paul McCarthy's avatar
Paul McCarthy committed
121

Paul McCarthy's avatar
Paul McCarthy committed
122
123
124
- `flake8 <http://flake8.pycqa.org/en/latest/>`_: This checks your code for
  adherence to the `PEP8 <https://www.python.org/dev/peps/pep-0008/>`_ coding
  standard.
125

Paul McCarthy's avatar
Paul McCarthy committed
126
127
128
- `pylint <https://www.pylint.org/>`_: This checks that your code follows
  other good conventions.

Paul McCarthy's avatar
Paul McCarthy committed
129
130
131

Because I like whitespace and vertical alignment more than PEP8 does, the
following violations of the PEP8 standard are accepted (see
Paul McCarthy's avatar
Paul McCarthy committed
132
`here <https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes>`_
Paul McCarthy's avatar
Paul McCarthy committed
133
134
for a list of error codes):

Paul McCarthy's avatar
Paul McCarthy committed
135
136
137
138
139
140
141
142
143
144
145
146
- E127: continuation line over-indented for visual indent
- E201: whitespace after '('
- E203: whitespace before ':'
- E221: multiple spaces before operator
- E222: multiple spaces after operator
- E241: multiple spaces after ','
- E271: multiple spaces after keyword
- E272: multiple spaces before keyword
- E301: expected 1 blank line, found 0
- E302: expected 2 blank lines, found 0
- E303: too many blank lines (3)
- E701: multiple statements on one line (colon)
Paul McCarthy's avatar
Paul McCarthy committed
147

Paul McCarthy's avatar
Paul McCarthy committed
148
149
150
151
152

The ``pylint`` tool can be *very* opinionated about how you write your code,
and also checks many of the same things as ``flake8``. So I disable all
refactoring and convention messages, and a few select warnings (type ``pylint
--list-msgs`` for a full list of codes):
Paul McCarthy's avatar
Paul McCarthy committed
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168

- W0511 (``fixme``): Warn about ``TODO`` and ``FIXME`` comments

- W0703 (``broad-except``): Warn about too-general ``except`` blocks (e.g.
  ``except Exception:``)

- W1202 (``logging-format-interpolation``): Warn about using ``format``
  when calling a log function, instead of using ``%`` string formatting.

To check code with ``flake8`` and ``pylint``, I use the following commands::


  flake8 --ignore=E127,E201,E203,E221,E222,E241,E271,E272,E301,E302,E303,E701 fsl
  pylint --extension-pkg-whitelist=numpy,wx \
         --generated-members=np.int8,np.uint8,np.int16,np.uint16,np.int32,np.uint32,np.int64,np.uint64,np.float32,np.float64,np.float128,wx.PyDeadObjectError \
         --disable=R,C,W0511,W0703,W1202 fsl