forked from GeoNode/geonode
-
Notifications
You must be signed in to change notification settings - Fork 0
/
review_criteria.txt
48 lines (26 loc) · 3.51 KB
/
review_criteria.txt
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
.. _review_criteria:
=====================
Patch Review criteria
=====================
When a patch is rejected in the [[Patch Review Process]], it should be given a valid review.
This review should point out issues with the patch where it fails to meet one or more of the following criteria.
#. Major new features must be approved by the Improvement Process
GeoNode needs to be coherent software despite the diverse interests driving its development. Therefor, major new features need to first be approved according to the [[Improvement Process]].
If a patch fails by this criterion, then its developer is welcome to go through the improvement process to get approval. Otherwise, they can refactor their patch into a GeoNode extension.
#. Patches need sufficient documentation
We strive to keep GeoNode well-documented. If a patch contributes significant functionality to GeoNode that requires documentation to be understood, the patch review is an opportunity to hold the developer accountable for providing the adequate documentation.
#. New functionality needs to be internationalized
We strive to build GeoNode in a way that can be used in many different localities, by all languages. While there is no localization requirement for GeoNode besides providing default English text, new user-facing features need to be sufficiently internationalized so that others can write translations.
#. Design consistency
We strive to keep the default user interface for GeoNode appealing for new users and developer's approaching the project. If a patch significantly diminishes the user experience of the software, then a patch may be rejected with a review of how to improve it.
.. note:: Good design can sometimes be in the eye of the beholder. Developer's are encouraged to consult the community and/or a designer about the user interface design of their patches, and to be humble in their design criticisms of others.
#. Code should be covered by automated tests
To make development easier for others and guarantee software quality, we strive to have good automated test coverage in GeoNode. Patches may fail a review for having insufficient unit and/or integration tests.
Reviews saying that a patch has insufficient tests should offer actionable advice on how to improve those tests. This advice could be to improve code coverage. It may also be a list of possible cases that currently lack tests.
#. Patches should not have known bugs
A patch may be rejected for having a known bug, (e.g.) one discovered by reading the code or testing it at the time of review.
#. Patches should meet GeoNode's code style guidelines
New patches should meet GeoNode's code style guidelines. We follow different conventions per language:
* In Java we use the GeoTools/GeoServer convention, essentially the [conventions recommended by Oracle](http://www.oracle.com/ technetwork/ java/codeconvtoc-136057.html) modified to make the recommended line length 100 columns instead of 80 to accommodate the long identifiers commonly used in GeoTools code. The GeoServer project provides an [Eclipse configuration](http://docs.geoserver.org/stable/en/developer/eclipse-guide/index.html#eclipse-preferences) which helps to stick to this convention.
* In Python we use the conventions enumerated in [PEP8](http://www.python.org/dev/peps/pep-0008/). Many editors have plugins available to assist with conformance to this convention.
* In JavaScript we use the OpenLayers conventions, described on the [OpenLayers wiki](http://trac.osgeo.org/openlayers/wiki/CodingStandards).