Skip to content
Xiangce Liu edited this page Apr 17, 2018 · 19 revisions

Submitting Contributions to the Project

Before contributing please review the contribution guide.

Before You Submit Your Pull Request

  • Ensure your commit message is useful and properly formatted.
  • There should optimally be only one commit. All of your intermediate work-in-progress commits should be squashed.
  • Include unit tests to cover both positive and negative cases utilizing reasonable examples of input data.
  • Run py.test to validate your code submission.
  • Run coverage tests to ensure that you are sufficiently testing your code:
    py.test --cov=insights
    
  • Run flake8 in the project root directory to check all files for PEP8 compliance. In insights project directory the file .flake8 configures the compliance checks that are performed.
  • Make sure your code is sufficiently documented following the Sphinx Google style for documentation. Include:
    • example input data;
    • an Examples: sections showing examples of use and attribute data structure;
    • clear description of how to use the code, assuming user has some knowledge of the domain; and
    • be clear on what “empty” data (falsey data objects) represents.
  • Check spelling in your documentation and comments:
    $ pylint --disable all --enable spelling --spelling-dict en_US --spelling-private-dict-file $HOME/my.dict module_filename.py
    You may have to install pylint, python-enchant and aspell-en in order to perform spell check. Also you will want to create your own dictionary ($HOME/my.dict) to add words like redhat that may not be in the default dictionary.
  • Check your documentation by running the generation script and verifying that the generated documentation looks correct and is accurate.
    $ cd docs
    $ make clean html_debug
    $ cd _build/html
    $ firefox index.html
  • You may also check your module documentation with $ pydoc module_name and the PEP 257 checking tool described in this article.
  • If you are exposing an attribute of type defaultdict or any other data structure that would mutate as a side effect of accessing the object, consider converting to a different type, for example dict.

After Submission of Your Pull Request

Once you have submitted your pull request a member of the project will review your submission. You may also add one or more of your team members as reviewers to your pull request. Continuous integration tests will be run on your code when the pull request has been submitted. The following tests will run:

  1. py.test will be executed for the project.
  2. Documentation will be generated to check for any errors.
  3. flake8 will be executed to check for PEP8 compliance.

If any of those tests fail you will receive a notification, and you will need to fix any problems before the pull request will be reviewed by the project team.

Review by the Project Team

One or more members of the project team will review your pull request and either provide feedback, or merge your request. If there are reviewers that have commented on your merge request you need to resolve those comments before the request can be merged.

If you make subsequent commits to the same pull request branch there is no need to squash subsequent commits, however multiple commits may be squashed when your request is merged. Upon successful merge the topic branch will be removed.

Checklist for Reviewers

  • Update the branch in Github to ensure it is up-to-date
  • Checkout pull request. See Resources [1]
  • Remove any pre-existing .pyc files from the project directory:
    $ find . -name '*.pyc' -delete
  • Run tests: py.test
  • Run coverage tests to determine test coverage for the changed code. Coverage should be 100% but there may be good reasons why it is not. Also having 100% coverage does not ensure testing is sufficient so be sure to review the tests.
    $ py.test --cov=insights
  • Generate the documentation and review the rendered HTML in a browser to check for problems.
  • Review code changes/additions to ensure the code follows good coding practices, is maintainable, and is performs the described functions.
  • Make sure only datasources, parsers, combiners, and associated bug fixes are accepted in PRs to master. No interface breaking changes are allowed as part of a regular release.
  • Review the test data and documentation to make sure no sensitive information, e.g. username, password, fqdn or public IP, is included.
  • Review the commit history to determine if the pull request needs to be squashed during the merge.
  • Add comments to the merge request if required.
  • Merge the request when all comments have been resolved. Use the option to Squash and Merge if the request includes multiple commits.

Resources

[1]: Consider using Hub for Git to make working with GitHub repos easier.

Clone this wiki locally