-
Notifications
You must be signed in to change notification settings - Fork 0
SubmissionChecklist
This document describes to contributors what we are looking for when we go through submitted patches. Please try to follow this as much as possible to save both the person reviewing your code as well as yourself some extra time.
Regardless of what you change it is recommended that you not only add unittests but also run the unittest_suite.py in the AUTOTEST_ROOT to be sure any changes you made did not break anything. Example:
[foo@bar autotest]$ utils/unittest_suite.py All passed!
Another tool we use to insure the correctness of our code is pylint. Due to the way imports have been implemented in the autotest code base a special wrapper is required to run pylint.
The file is located in $AUTOTEST_ROOT/utils/run_pylint.py
Simply run the command from your code directory and the rest is taken care of.
Example of running on a source file with warnings:
[lmr@freedom autotest]$ utils/run_pylint.py client/job.py ************* Module client.job E1002: 93,4:base_client_job.__init__: Use of super on an old style class E0203:886,18:base_client_job.reboot: Access to member 'last_boot_tag' before its definition line 888
I've looked and those were false alarms.
Same thing, now with an error I introduced:
[lmr@freedom autotest]$ utils/run_pylint.py client/job.py ************* Module client.job E1002: 93,4:base_client_job.__init__: Use of super on an old style class E0602:175,14:base_client_job._pre_record_init: Undefined variable 'bar' E0203:887,18:base_client_job.reboot: Access to member 'last_boot_tag' before its definition line 889
Here is the error, an undefined variable:
[lmr@freedom autotest]$ git diff
diff --git a/client/job.py b/client/job.py
index c5e362b..8d335b4 100644
--- a/client/job.py
+++ b/client/job.py
@@ -172,6 +172,7 @@ class base_client_job(base_job.base_job):
As of now self.record() needs self.resultdir, self._group_level,
self.harness and of course self._logger.
"""
+ foo = bar
if not options.cont:
self._cleanup_debugdir_files()
self._cleanup_results_dir()
So, pylint is a valuable ally here, use it!
Yet another tool that we use to fix indentation inconsistencies
(important thing to notice when you're doing python code) is
$AUTOTEST_ROOT/utils/reindent.py. You can use the script giving
your files as an argument, so it will prune trailing whitespaces
from lines and fix incorrect indentation.
- Submit a separate patch for each logical change (if your description includes "add this, fix that, remove three other unrelated things"; probably bad).
- Put a summary line at the very top of the commit message, explaining briefly what has changed and where.
- Put cleanups in separate patches than functional changes.
- Please set up your git environment properly, and always sign your patches using commit -s.
Patch descriptions need to be as verbose as possible. Some of these points are obvious but still worth mentioning. Describe:
- The motivation for the change - what problem are you trying to fix?
- High level description / design approach of how your change works (for non-trivial changes)
- Implementation details
- Testing results
All tests must follow the control file specification Refer to the Control Requirements section
Autotest has its own unique coding style, please familiarize yourself with the following document: Refer to the coding style documentation
Please make sure you have git properly setup. We have a fairly brief and descriptive document explaining how to get the basics setup here. In particular, tend to stick to one version of your written name, so all your contributions appear under a same name on git shortlog. For example:
John Doe Silverman
or
John D. Silverman
Please do choose between one of them when sending patches, for consistency.
Please, pretty please, do send your patches with git-send-email, otherwise they tend to get scrambled and not applicable. Or, send github pull requests. Github has a fairly nice documentation on how to do it here.
In general, if you are not feeling like doing a pull request for a single patch, fine, use the mailing list. But for bigger work, pull requests are a good way of doing things.
commit 37fe66bb2f6d0b489d70426ed4a78953083c7e46 Author: Nishanth Aravamudan <[email protected]> Date: Thu Apr 26 03:38:44 2012 +0000 conmux/ivm: use immediate reboot rather than delayed Delayed reboots use EPOW, which does not always result in a shutdown of the LPAR. Use the more sever immediate shutdown, to ensure the LPAR goes down. This matches the HMC code. Signed-off-by: Nishanth Aravamudan <[email protected]>