Skip to content

CONTRIBUTING: add some initial guidelines #6485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
# Issues

We welcome reports of technical issues with the components of the xen-api
toolstack. Please make sure that the description of the issue is as detailed as
possible to help anyone investigating it:

1) Mention how it was detected, if and how it could be reproduced

1) What's the desired behaviour? In what cases would it be useful?

1) Include error messages, related logs if appropriate

# Pull Requests

To contribute changes to xen-api, please fork the repository on
GitHub, and then submit a pull request.

It is required to add a `Signed-off-by:` as a
[Developers Certificate of Origin](http://developercertificate.org).
It certifies the patch's origin and is licensed under an
appropriate open-source licence to include it in Xapi:
https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---signoff

The following points are intended to describe what makes a contribution "good" -
easier to review, integrate, and mantain. Please follow them in your work.

## Commit subjects and PR titles

Commit subjects should preferrably start with the name of the component the
commit is most related to, and describe what the commit achieves. If your
commit only touches the `ocaml/xenopsd` directory, it should look like this,
for example:

```
xenopsd: Fix a deadlock during VM suspend
```

Similar principle applies to Pull Request titles. If there is only a single
commit in the PR, Github will automatically copy its subject and description to
the PR's title and body. If there are several commits in the PR, describe what
the PR achieves and the components it most directly impacts.

If the commit subject includes some tracking identifier (such as `CP-1234`, for
example) referring to internal systems, please make sure to include all of the
essential information in the public descriptions - describe the symptoms of the
issue, how it was detected, investigated, how it could be reproduced, what are
the trade-offs and so on as appropriate.

## Split into commits

Following from the rules described above, if what the commit achieves is
difficult to fit into its subject, it is probably better to split it into
several commits, if possible. Note that every commit should build (`make`
should work and the CI should pass) independently, without requiring future
commits. This means some modifications can't really be split into several
commits (datamodel changes, in particular, require modifications to several
components at the same time), but makes it easier to revert part of the Pull
Request if some issues are detected in integration testing at a later point.

## Good Commit Messages

Commit messages (and the body of a Pull Request) should be as helpful and
descriptive as possible. If applicable, please include a description of current
behaviour, your changes, and the new behaviour. Link any appropriate
documentation, issues, or commits.

## CI

Please make sure your Pull Request passes the Github CI. It will verify that
your code has been properly formatted (can be done locally with `make format`),
builds (`make` and `make check`), and passes the unit tests (`make test`).
The CI will run in the branches of your fork, so you can verify it passes
there before opening a Pull Request.

## Testing

Describe what kind of testing your contribution underwent. If the testing was
manual, please describe the commands or external clients that were used. If the
tests were automated, include at least a cursory description/name of the tests,
when they were regressed, if possible.

Please note that any contribution to the code of the project will likely
require at least some testing to be done. Depending on how central the
component touched in your PR is to the system, the more things could only be
detected in real-world usecases through integration testing.

If you are still waiting on some testing to be done, please mark the PR as a
"draft" and make the reasoning clear.

If your contribution doesn't intend to have any functional changes, please make
that clear as well.

## Feature work

If your contribution adds some new feature or reworks some major aspect of the
system (as opposed to one-off fixes), it can be benefitial to first describe
the plan of your work in a design proposal. Architectural issues are better
spotted early on, and taking a big-picture view can often lead to new insights.

An example of a design proposal is here:

https://github.com/xapi-project/xen-api/pull/6387

If submitting a design first is not possible, include documentation alongside
with your PR describing the work, like it was done in the last three commits
here:

https://github.com/xapi-project/xen-api/pull/6457

Note that the design will often serve as documentation as well - so take care
updating it after the implementation is done to better reflect reality.

## Review process and merge

It can often be useful to address review suggestions with a "fixup" commit
(created manually or with the help of `git commit --amend=HASH`). This way it
is clear what the original code was and what your fix touches. Once the
fixup commit has been reviewed and the PR approved, please squash the fixup
commits with `git rebase --autosquash` before merging. Otherwise the commits in
the Pull Request should stay as independent commits - we do not require
squashing all the commits into a single one on merge.
3 changes: 3 additions & 0 deletions README.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ It certifies the patch's origin and is licensed under an
appropriate open-source licence to include it in Xapi:
https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---signoff

For more detailed guidelines on what makes a good contribution, see
[CONTRIBUTING](./CONTRIBUTING.md).

Discussions
-----------

Expand Down
Loading