summaryrefslogtreecommitdiff
path: root/CONTRIBUTING.rst
diff options
context:
space:
mode:
authorAmrith Kumar <amrith@tesora.com>2016-09-19 08:26:24 -0400
committerPeter Stachowski <peter@tesora.com>2016-09-19 21:04:14 +0000
commita2d336de2a2275b6abd6e7933347af72363bcc03 (patch)
tree4f7fdfbd38fe67e27c3e86d4672eec6bd148ac7b /CONTRIBUTING.rst
parentd7fc09a1bc67d72aa22f58baff5379cd33c4f7a5 (diff)
downloadtrove-a2d336de2a2275b6abd6e7933347af72363bcc03.tar.gz
improve pylint; generate errors and config in sorted order
Since the config is not in a deterministic order makes it hard to compare two config's and see what changed. Personally, I'm not positive I understand this use-case; i.e. you have an existing config file, you save it, and then rebuild and then diff the two files. I'd have thought you'd just run check and the output of the tool was the diff. I however do see the value in sorting the file so that when someone submits a change that includes a change to the config, reviewers can see more easily what the change is doing. Similarly, the output from pylint (errors) are generated one file at a time and os.walk makes no guarantee of deterministic order. So we should collect all errors (across all files) and then print an ordered list for human consumption. The intent is also to make pylint voting soon (in master). the changes to contributing.rst and tox.ini are to make that easier. The config file has also been sorted in place. This change was motivated by an email exchange with Peter so I am marking him as a co-conspirator. The line numbers were removed from the tools/trove-pylint.config file as these would change whenever the line numbers in the file changed (since they are currently not being used in the comparison; they can be re-added if deemed necessary at the cost of having every 'rebuild' run create a different file). The tools/trove-pylint.config was regenerated as well, since the remaining two errors seem to be innocuous: ERROR: trove/taskmanager/manager.py 392: E1101 no-member, Manager.upgrade: Instance of 'BuiltInstance' has no 'upgrade' member (new method introduced by instance upgrade; other BuiltInstance member errors are already ignored.) and ERROR: trove/guestagent/datastore/experimental/postgresql/service/ access.py 80: E1101 no-member, PgSqlAccess.list_access: Instance ofi 'PgSqlAccess' has no '_find_user' member (this is due to the fact that PostgreSQL is spread over multiple files and pylint should cease to complain once https://review.openstack.org/#/c/346082/ lands.) Change-Id: I910c738d3845b7749e57910f76523150ec5a5bff Closes-Bug: #1625158 Closes-Bug: #1625245 Co-Authored-By: Peter Stachowski <peter@tesora.com>
Diffstat (limited to 'CONTRIBUTING.rst')
-rw-r--r--CONTRIBUTING.rst62
1 files changed, 59 insertions, 3 deletions
diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index a9864b8b..68abe04c 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -46,7 +46,6 @@ The Trove project encourages the guidelines (below).
* It is your opinion that the change, as proposed, should be
considered for merging.
-
- A rating of 0 on a code review is indicated if:
* The reason why you believe that the proposed change needs
@@ -71,8 +70,9 @@ The Trove project encourages the guidelines (below).
* The subject matter of the change (not the commit message)
violates some well understood OpenStack procedure(s),
* The change contains content that is demonstrably inappropriate,
- * The test cases do not exercise the change(s) being proposed.
-
+ * The test cases do not exercise the change(s) being proposed,
+ * The change causes a failure in the pylint job (see pylint
+ section below).
Some other reviewing guidelines:
@@ -89,6 +89,7 @@ Other references:
- https://wiki.openstack.org/wiki/GitCommitMessages
- http://docs.openstack.org/developer/hacking/
- https://review.openstack.org/#/c/116176/
+ - trove-pylint readme file in tools/trove-pylint.README
Approving changes
-----------------
@@ -152,6 +153,61 @@ The generated documentation is found::
api-ref/html/index.html
+Trove PyLint Failures
+=====================
+
+The Trove project uses trove-pylint (tools/trove-pylint) in the gate
+and this job is intended to help catch coding errors that sometimes
+may not get caught in a code review, or by the automated tests.
+
+The gate-trove-tox-pylint jobs are run by the CI, and these invoke the
+command in tools/trove-pylint.
+
+The tool can produce false-positive notifications and therefore
+supports a mechanism to provide a list of errors that are to be
+ignored.
+
+Before submitting a change, please do run
+
+.. code-block:: bash
+
+ $ tox -e pylint
+
+on your development environment. If this fails, you will have to
+resolve all the errors before you can commit the code.
+
+This means you either must fix the problem being identified, or
+regenerate the list of ignored errors and submit that as part of your
+review.
+
+To regenerate the list of ignored errors, you run the command(s):
+
+.. code-block:: bash
+
+ $ tox -e pylint rebuild
+
+Warning: trove-pylint is very sensitive to the version(s) of pylint
+and astroid that are installed on your system and for this reason, a
+tox environment is provided that will mimic the environment that
+pylint will encouter in the gate.
+
+Pre-commit checklist
+====================
+
+Before commiting code to Gerrit for review, please at least do the
+following on your development system and ensure that they pass.
+
+.. code-block:: bash
+
+ $ tox -e pep8
+ $ tox -e py27
+ $ tox -e py34
+ $ tox -e pylint
+
+If you are unable to get these to pass locally, it is a waste of the
+CI resources to push up a change for review.
+
+
Testing
=======