Bimba Shrestha 1b1cc3d1b7 adding some static-anslysis docs

2020-05-07 10:38:06 -05:00

9.2 KiB

Raw Blame History

Contributing to Zstandard

We want to make contributing to this project as easy and transparent as possible.

Our Development Process

New versions are being developed in the "dev" branch, or in their own feature branch. When they are deemed ready for a release, they are merged into "master".

As a consequences, all contributions must stage first through "dev" or their own feature branch.

Pull Requests

We actively welcome your pull requests.

Fork the repo and create your branch from dev.
If you've added code that should be tested, add tests.
If you've changed APIs, update the documentation.
Ensure the test suite passes.
Make sure your code lints.
If you haven't already, complete the Contributor License Agreement ("CLA").

Contributor License Agreement ("CLA")

In order to accept your pull request, we need you to submit a CLA. You only need to do this once to work on any of Facebook's open source projects.

Complete your CLA here: https://code.facebook.com/cla

Workflow

Zstd uses a branch-based workflow for making changes to the codebase. Typically, zstd will use a new branch per sizable topic. For smaller changes, it is okay to lump multiple related changes into a branch.

Our contribution process works in three main stages:

Local development

Update:

Checkout your fork of zstd if you have not already

git checkout https://github.com/<username>/zstd
cd zstd

Update your local dev branch

git pull https://github.com/facebook/zstd dev
git push origin dev

Topic and deveopment:

Make a new branch on your fork about the topic you're developing for

# branch names should be consise but sufficiently informative
git checkout -b <branch-name>
git push origin <branch-name>

Make commits and push

# make some changes =
git add -u && git commit -m <message>
git push origin <branch-name>

Note: run local tests to ensure that your changes didn't break existing functionality
- Quick check
```
make shortest
```
- Longer check
```
make test
```

Code Review and CI tests
- Ensure CI tests pass:
  - Before sharing anything to the community, make sure that all CI tests pass on your local fork. See our section on setting up your CI environment for more information on how to do this.
  - Ensure that static analysis passes on your development machine. See the Static Analysis section below to see how to do this.
- Create a pull request:
  - When you are ready to share you changes to the community, create a pull request from your branch to facebook:dev. You can do this very easily by clicking 'Create Pull Request' on your fork's home page.
  - From there, select the branch where you made changes as your source branch and facebook:dev as the destination.
  - Examine the diff presented between the two branches to make sure there is nothing unexpected.
- Write a good pull request description:
  - While there is no strict template that our contributers follow, we would like them to sufficiently summarize and motivate the changes they are proposing. We recommend all pull requests, at least indirectly, address the following points.
    - Is this pull request important and why?
    - Is it addressing an issue? If so, what issue? (provide links for convenience please)
    - Is this a new feature? If so, why is it useful and/or necessary?
    - Are there background references and documents that reviewers should be aware of to properly assess this change?
  - Note: make sure to point out any design and architectural decisions that you made and the rationale behind them.
  - Note: if you have been working with a specific user and would like them to review your work, make sure you mention them using (@)
- Submit the pull request and iterate with feedback.
Merge and Release
- Getting approval:
  - You will have to iterate on your changes with feedback from other collaborators to reach a point where your pull request can be safely merged.
  - To avoid too many comments on style and convention, make sure that you have a look at our style section below before creating a pull request.
  - Eventually, someone from the zstd team will approve your pull request and not long after merge it into the dev branch.
- Housekeeping:
  - Most PRs are linked with one or more Github issues. If this is the case for your PR, make sure the corresponding issue is mentioned. If your change 'fixes' or completely addresses the issue at hand, then please indicate this by requesting that an issue be closed by commenting.
  - Just because your changes have been merged does not mean the topic or larger issue is complete. Remember that the change must make it to an official zstd release for it to be meaningful. We recommend that contributers track the activity on their pull request and corresponding issue(s) page(s) until their change makes it to the next release of zstd. Users will often discover bugs in your code or suggest ways to refine and improve your initial changes even after the pull request is merged.

Static Analysis

Static analysis is a process for examining the correctness or validity of a program without actually executing it. It usually helps us find many simple bugs. Zstd uses clang's scan-build tool for static analysis. You can install it by following the instructions for your OS on https://clang-analyzer.llvm.org/scan-build.

Once installed, you can ensure that our static analysis tests pass on your local development machine by running:

make staticAnalyze

In general, you can use scan-build to static analyze any build script. For example, to static analyze just contrib/largeNbDicts and nothing else, you can run:

scan-build make -C contrib/largeNbDicts largeNbDicts

Setting up continuous integration (CI) on your fork

Zstd uses a number of different continuous integration (CI) tools to ensure that new changes are well tested before they make it to an official release. Specifically, we use the platforms travis-ci, circle-ci, and appveyor.

Changes cannot be merged into the main dev branch unless they pass all of our CI tests. The easiest way to run these CI tests on your own before submitting a PR to our dev branch is to configure your personal fork of zstd with each of the CI platforms. Below, you'll find instructions for doing this.

travis-ci

Follow these steps to link travis-ci with your github fork of zstd

Make sure you are logged into your github account
Go to https://travis-ci.org/
Click 'Sign in with Github' on the top right
Click 'Authorize travis-ci'
Click 'Activate all repositories using Github Apps'
Select 'Only select repositories' and select your fork of zstd from the drop down
Click 'Approve and Install'
Click 'Sign in with Github' again. This time, it will be for travis-pro (which will let you view your tests on the web dashboard)
Click 'Authorize travis-pro'
You should have travis set up on your fork now.

circle-ci

TODO

appveyor

Follow these steps to link circle-ci with your girhub fork of zstd

Make sure you are logged into your github account
Go to https://www.appveyor.com/
Click 'Sign in' on the top right
Select 'Github' on the left panel
Click 'Authorize appveyor'
You might be asked to select which repositories you want to give appveyor permission to. Select your fork of zstd if you're prompted
You should have appveyor set up on your fork now.

General notes on CI

CI tests run every time a pull request (PR) is created or updated. The exact tests that get run will depend on the destination branch you specify. Some tests take longer to run than others. Currently, our CI is set up to run a short series of tests when creating a PR to the dev branch and a longer series of tests when creating a PR to the master branch. You can look in the configuration files of the respective CI platform for more information on what gets run when.

Most people will just want to create a PR with the destination set to their local dev branch of zstd. You can then find the status of the tests on the PR's page. You can also re-run tests and cancel running tests from the PR page or from the respective CI's dashboard.

Issues

We use GitHub issues to track public bugs. Please ensure your description is clear and has sufficient instructions to be able to reproduce the issue.

Facebook has a bounty program for the safe disclosure of security bugs. In those cases, please go through the process outlined on that page and do not file a public issue.

Coding Style

4 spaces for indentation rather than tabs

License

By contributing to Zstandard, you agree that your contributions will be licensed under both the LICENSE file and the COPYING file in the root directory of this source tree.

9.2 KiB Raw Blame History