Skip to content
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

Update contributing guide #1876

Closed
wants to merge 7 commits into from
Closed
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
165 changes: 82 additions & 83 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@ us assess your changes faster and makes it easier for us to merge your
submission!

There are many ways to contribute: writing tutorials or blog posts about your
experience, improving the [documentation], submitting bug reports, answering
questions on the [mailing list], or writing code which can be incorporated into
experience, improving the [documentation], submitting bug reports, answering
questions on the [mailing list], or writing code which can be incorporated into
Archivematica itself.

<!-- START doctoc generated TOC please keep comment here to allow auto update -->
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
**Table of Contents**

- [Submitting bugs](#submitting-bugs)
- [Submitting enhancement ideas](#submitting-enhancements)
- [Submitting enhancement ideas](#submitting-enhancement-ideas)
- [Submitting code changes](#submitting-code-changes)
- [Permalinks](#permalinks)
- [Getting started](#getting-started)
- [When to submit code for review?](#when-to-submit-code-for-review)
- [Opening the pull request](#opening-the-pull-request)
Expand All @@ -37,22 +38,23 @@ Archivematica itself.
- [Commit History](#commit-history)
- [Commits should be specific and atomic](#commits-should-be-specific-and-atomic)
- [Every commit should work](#every-commit-should-work)
- [Commit messages](#commit-messages)
- [Commit summaries should be concise](#commit-summaries-should-be-concise)
- [Commit messages should be as detailed as they need to be (and no more)](#commit-messages-should-be-as-detailed-as-they-need-to-be-and-no-more)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->

## Submitting bugs

If you find a security vulnerability, do NOT open an issue. Email
[email protected] instead.
If you find a security vulnerability, do NOT open an issue. See our
[Security Policy] instead.

Issues can be filed using GitHub Issues in the [Archivematica Issues repo].
It is recommended to file issues there rather than in any of the
Archivematica-related code repositories. Artefactual staff also use GitHub
Issues can be filed using GitHub Issues in the [Archivematica Issues repo].
It is recommended to file issues there rather than in any of the
Archivematica-related code repositories. Artefactual staff also use GitHub
issues for any work they do on the Archivematica project.

You can also post in our [user] mailing list. A post to the mailing list is
You can also post in our user [mailing list]. A post to the mailing list is
always welcome, especially if you're unsure if it's a bug or a local problem!

Useful questions to answer if you're having problems include:
Expand All @@ -70,12 +72,12 @@ Useful questions to answer if you're having problems include:
## Submitting enhancement ideas

Similar to submitting bugs, you are welcome to submit ideas for enhancements or
new features in the [Archivematica Issues repo]. This is also where Artefactual
new features in the [Archivematica Issues repo]. This is also where Artefactual
staff record upcoming enhancements when they have been sponsored for inclusion
either by Artefactual Systems or by a client.

Please feel free also to use the [Issues repo wiki] as a space for gathering and
collaborating on ideas. If you are not already a member of the
Please feel free also to use the [Issues repo wiki] as a space for gathering and
collaborating on ideas. If you are not already a member of the
Archivematica repo (required for editing the wiki), file an issue there with
the title "Request membership."

Expand All @@ -87,16 +89,15 @@ outside contributors.

Here's an outline of the contribution process:

1. File an issue in the
[Archivematica Issues repo].
1. File an issue in the [Archivematica Issues repo].
2. Fork the Artefactual project on GitHub, and commit your changes to a branch
in your fork.
3. Open a pull request.
4. Back and forth discussion with developers on the branch.
5. Make any changes suggested by reviewers.
6. Repeat 3 and 4 as necessary.
7. Clean up commit history, if necessary.
8. Sign a Contributor's Agreement, if you haven't already.
3. Open a pull request using `qa/1.x` as the base branch.
4. Sign a [Contributor's Agreement], if you haven't already.
5. Back and forth discussion with developers on the branch.
6. Make any changes suggested by reviewers.
7. Repeat 5 and 6 as necessary.
8. Clean up commit history, if necessary.
9. Your branch will be merged!

### Permalinks
Expand All @@ -114,7 +115,7 @@ So you have something to contribute to an Artefactual project. Great!

To install Archivematica, see our [development installation] instructions.

Artefactual uses [GitHub]'s pull request feature for code review. Every change
Artefactual uses [GitHub]'s pull request feature for code review. Every change
being submitted to an Artefactual project should be
submitted as a pull request to the appropriate repository. A branch being
submitted for code review should contain commits covering a related section of
Expand Down Expand Up @@ -181,73 +182,72 @@ The Archivematica contributor's agreement is based almost verbatim on the
[Apache Foundation]'s individual [contributor license].

If you have any questions or concerns about the Contributor's Agreement,
please email us at [email protected] to discuss them.
please email us at [[email protected]](mailto:[email protected])
to discuss them.

### Why do I have to sign a Contributor's Agreement?

One of the key challenges for open source software is to support a collaborative
development environment while protecting the rights of contributors and users
development environment while protecting the rights of contributors and users
over the long-term.
Unifying Archivematica copyrights through contributor agreements is the best way
to protect the availability and sustainability of Archivematica over the
to protect the availability and sustainability of Archivematica over the
long-term as free and open-source software.
In all cases, contributors who sign the Contributor's Agreement retain full
rights to use their original contributions for any other purpose outside of
Archivematica, while enabling Artefactual Systems, any successor organization
which may eventually take over responsibility for Archivematica, and the wider
Archivematica community to benefit from their collaboration and contributions
In all cases, contributors who sign the Contributor's Agreement retain full
rights to use their original contributions for any other purpose outside of
Archivematica, while enabling Artefactual Systems, any successor organization
which may eventually take over responsibility for Archivematica, and the wider
Archivematica community to benefit from their collaboration and contributions
in this open source project.

[Artefactual Systems] has made the decision and has a proven track record of
[Artefactual Systems] has made the decision and has a proven track record of
making our intellectual property available to the community at large.
By standardizing contributions on these agreements the Archivematica
By standardizing contributions on these agreements the Archivematica
intellectual property position does not become too complicated.
This ensures our resources are devoted to making our project the best they can
This ensures our resources are devoted to making our project the best they can
be, rather than fighting legal battles over contributions.

### How do I send in an agreement?

Please read and sign the [Contributor's Agreement] and email it to
[email protected].
Please read and sign the [Contributor's Agreement] and email it to
[[email protected]](mailto:[email protected]).

Alternatively, you may send a printed, signed agreement to:

Artefactual Systems Inc.
201 - 301 Sixth Street
New Westminster BC V3L 3A7
#1 -10318 Whalley Blvd.
Surrey, BC V3T 4H4
Canada

## Contribution standards

### Style

Archivematica uses the Python [PEP8] community style guidelines. Newly-written
code should conform to PEP-8 style. PEP8 is a daunting document, but there are
Archivematica uses the Python [PEP8] community style guidelines. Newly-written
code should conform to PEP-8 style. PEP8 is a daunting document, but there are
very good linters available that you can run to check style in your code.

* The [Black] tool formats the code automatically. The output is deterministic
* The [Black] tool formats the code automatically. The output is deterministic
for any given input. Editor integration is possible.

* The [flake8] tool checks for style problems as well as errors and complexity.
It can be used at the command line or as a plugin in your preferred text
editor/IDE. The Archivematica [continuous integration system] will currently
* The [flake8] tool checks for style problems as well as errors and complexity.
It can be used at the command line or as a plugin in your preferred text
editor/IDE. The Archivematica [continuous integration system] will currently
check code for compliance against flake8.

We have integrated these tools with our CI, i.e. pull requests will fail to
build when the tools above report errors.

Additionally [Pylint] is used by developers internally at Artefactual to
Additionally [pre-commit] is used by developers internally at Artefactual to
highlight other potential areas of improvement during code-review.

#### Some extra notes

A few additional stylistic preferences might not get flagged by linters:

* Don't use variable or parameter names that shadow builtin functions and
types. For example, don't name a variable "file". (Unfortunately, Python uses
types. For example, don't name a variable "id". (Unfortunately, Python uses
many useful names for its builtin types and functions.)
* Sort imports alphabetically within their grouping to reduce duplicate
imports.

#### Exceptions

Expand All @@ -274,36 +274,28 @@ a few PEP8 rules in order to match existing code. In particular:
it's okay to make your new function and its parameters camelCase to match.
Try to use snake_case internally, however.

You should try to write Python 2 / Python 3 compatible code where possible.
Using `from __future__ import print_function, division, absolute_import` will
help with this. Some libraries run in Python 2 and Python 3 already; this
behaviour should be maintained.

### Documentation

New classes and functions should generally be documented using
[docstrings]; these help in providing clarity, and can also be used to generate
API documentation later. Generally any function which isn't obvious
(any function longer than a line or two) should have a docstring.
[docstrings]; these help in providing clarity, and can also be used to generate
API documentation later. Generally any function which isn't obvious
(any function longer than a line or two) should have a docstring.
When in doubt: document! Python's [PEP 257] document provides a useful
guideline for docstring style. Generally, prefer using
guideline for docstring style. Generally, prefer using
[Sphinx-compatible docstrings]. More [examples] and [attributes to use] can
be found on the Sphinx website.

### Tests

New code should also have unit tests. Tests are written in [unittest] style
and run with [py.test]. For tests requiring the Django ORM, we use the
Django-provided[TestCase], which extends `unittest.TestCase`.
New code should also have unit tests. Tests are written in [unittest] style
and run with [pytest]. For tests requiring the Django ORM, we use
[pytest-django].

Tests are found in the `tests` directory, a sibling of the directory containing
the code. `test_foo.py` contains tests for `foo.py`. For clarity, group tests
for the same function and similar tests into the same class within that file.
This will also allow you to share setup and teardown code.

If you are testing code that makes HTTP requests, using [VCR.py] is highly
recommended. It should already be a development dependency.

### Commit History

A clean commit history makes it easier to see what changes are related, and why
Expand Down Expand Up @@ -359,21 +351,29 @@ and b) what the change is. For example:

Clear commit summary:

> Replace 404 messages with a user-friendly one
```
Replace 404 messages with a user-friendly one
```

Unclear commit summaries:

> Fixed some normalization bugs
```
Fixed some normalization bugs
```

> Bugfixes
```
Bugfixes
```

The unclear messages make it hard to tell at a glance what changed, and that
makes browsing the commit history harder.

A commit message should use the [imperative mood] which should always be able to
complete the following sentence:

If applied, this commit will <your subject line here>
```
If applied, this commit will <your subject line here>
```

#### Commit messages should be as detailed as they need to be (and no more)

Expand Down Expand Up @@ -404,39 +404,38 @@ two together.

Further content comes after a blank line.
```

[documentation]: https://github.com/artefactual/archivematica-docs/
[mailing list]: https://groups.google.com/forum/#!forum/archivematica
[Security Policy]: SECURITY.md
[Archivematica Issues repo]: https://github.com/archivematica/Issues
[user]: https://groups.google.com/forum/#!forum/archivematica
[Archivematica Issues repo]: https://github.com/archivematica/Issues
[Issues repo wiki]: https://github.com/archivematica/Issues/wiki
[Issues repo wiki]: https://github.com/archivematica/Issues/wiki
[files]: https://help.github.com/articles/getting-permanent-links-to-files/
[code snippets]: https://help.github.com/articles/creating-a-permanent-link-to-a-code-snippet/
[development installation]: https://wiki.archivematica.org/Getting_started#Installation
[development installation]: https://github.com/artefactual/archivematica/tree/qa/1.x/hack#archivematica-development-on-docker-compose
[GitHub]: https://github.com/
[guide]: https://help.github.com/articles/fork-a-repo
[excellent]: https://help.github.com/articles/using-pull-requests
[Line comment]: http://i.imgur.com/FsWppGN.png
[Line comment]: https://i.imgur.com/FsWppGN.png
[code review guidelines]: code_review.md
[interactive rebase feature]: http://www.git-scm.com/book/en/Git-Tools-Rewriting-History
[interactive rebase feature]: https://www.git-scm.com/book/en/Git-Tools-Rewriting-History
[Contributor's Agreement]: https://wiki.archivematica.org/images/e/e6/Archivematica-CLA-firstname-lastname-YYYY.pdf
[Apache Foundation]: http://apache.org
[contributor license]: http://www.apache.org/licenses/icla.txt
[Artefactual Systems]: http://artefactual.com
[Apache Foundation]: https://apache.org
[contributor license]: https://www.apache.org/licenses/icla.txt
[Artefactual Systems]: https://artefactual.com
[PEP8]: https://www.python.org/dev/peps/pep-0008/
[Black]: https://github.com/ambv/black
[flake8]: https://pypi.python.org/pypi/flake8
[continuous integration system]: https://travis-ci.org/artefactual/archivematica
[Pylint]: https://www.pylint.org/
[continuous integration system]: https://github.com/artefactual/archivematica/actions
[pre-commit]: https://pre-commit.com/
[docstrings]: https://en.wikipedia.org/wiki/Docstring#Python
[PEP 257]: https://www.python.org/dev/peps/pep-0257/
[Sphinx-compatible docstrings]: http://pythonhosted.org/an_example_pypi_project/sphinx.html#function-definitions
[examples]: http://sphinx-doc.org/domains.html#info-field-lists
[attributes to use]: http://sphinx-doc.org/domains.html#the-python-domain
[unittest]: https://docs.python.org/2/library/unittest.html
[py.test]: http://pytest.org
[TestCase]: https://docs.djangoproject.com/en/1.8/topics/testing/tools/#django.test.TestCase
[VCR.py]: https://github.com/kevin1024/vcrpy
[Sphinx-compatible docstrings]: https://pythonhosted.org/an_example_pypi_project/sphinx.html#function-definitions
[examples]: https://www.sphinx-doc.org/en/master/#info-field-lists
[attributes to use]: https://www.sphinx-doc.org/en/master/#the-python-domain
[unittest]: https://docs.python.org/3.9/library/unittest.html
[pytest]: https://docs.pytest.org/
[pytest-django]: https://pytest-django.readthedocs.io/en/latest/database.html
[How to Write a Git Commit Message]: https://chris.beams.io/posts/git-commit/
[seven rules of a great Git commit message]: https://chris.beams.io/posts/git-commit/#seven-rules
[imperative mood]: https://chris.beams.io/posts/git-commit/#imperative
Loading