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

[Docs] Update and rework CONTRIBUTING.md #3859

Merged
merged 20 commits into from
Sep 17, 2018
152 changes: 111 additions & 41 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,47 +1,117 @@
# Contributing to Loris

We'd love to have you contribute to Loris. The first thing you should do
before contributing is probably sign up for the [LORIS developers' mailing list](http://www.bic.mni.mcgill.ca/mailman/listinfo/loris-dev).
We'd love to have you contribute to Loris. The first thing you should
do before contributing is probably sign up for the [LORIS developers'
mailing list](http://www.bic.mni.mcgill.ca/mailman/listinfo/loris-dev).

Your next step before issuing a pull request is to review our
[Coding Standards](./docs/CodingStandards.md). If you are doing
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coding standards link appears to be broken.

front-end development you should also check out our [React
guidelines](./LORIS_react.README.md).

You can also learn about our code
review process by perusing our [Code Review
Checklist](https://github.com/aces/Loris/wiki/Code-Review-Checklist)
in the [LORIS Wiki](https://github.com/aces/Loris/wiki). These will be
some of the factors we'll consider when reviewing your code.

## Development branches

Pull requests should be based on and sent to the VERSIONNUMBER-dev branch of the version that you are targeting. The master branch is reserved for stable releases.

Your current version number can be found in the VERSION file under the LORIS root.

## Code Contributions

If you'd like to contribute code, here are some things to keep in mind.

* If your changes require any table modifications, don't forget to modify the
SQL/0000*.sql file(s) with your changes for new installs, and also
include a patch for existing projects to apply to get your changes of which
should be placed in the corresponding SQL/VERSIONNUMBER/ directory.
* Include a test for any new module in the modules/MODULENAME/test/
directory. You can look at other modules for examples of how to write tests.
If you have questions, feel free to mail the mailing list.
* Add your new tests to get auto-run by Travis in the .travis.yml to make sure that
other people don't accidentally break your module.
* Check out our Coding Standards in the [docs/ directory](https://github.com/aces/Loris/tree/master/docs) and also our [Code Review Checklist](https://github.com/aces/Loris/wiki/Code-Review-Checklist) in the [GitHub Wiki](https://github.com/aces/Loris/wiki) and the Pull Request guidelines Readme in this directory.
* Try and make sure you run PHP codesniffer using the standards file in
docs/LorisCS.xml before sending any pull request, otherwise the Loris tests may
fail and we won't be able to merge your pull request.
* Keep in mind that people are currently using Loris so try and make any changes
backwards-compatible with existing installations. If you must change something
in a non-backwards-compatible way, document it in your pull request description and
tag it with "Caveat For Existing Projects" so that we know that the change needs
to be mentioned in release notes. Non-backwards-compatible changes should be sent
to the next major release (e.g. 17.X -> 18.0) while backwards-compatible changes should
be sent to the next minor release (e.g. 17.1.0 -> 17.2.0) and backwards-compatible bug
fixes should be sent to the next minor release update (e.g. 17.1.0 -> 17.1.1)

## Ways To Get Started

If you're looking for ideas for a way to contribute but don't know where to get
started, some ideas to get you started:

* You can browse some of our public [Issues](https://github.com/aces/Loris/issues)
You should base your pull requests on one of the following branches
depending on the kind of change you are making:

#### Bug Fixes
- Branch: `bugfix` - Label:
![](https://via.placeholder.com/15/cc9966/000000?text=+) **[branch]
bugfix** - Content: Generally these changes do not require SQL scripts
and are concise with the sole objective to correct a single problem
in the code.

#### Minor Changes and Small Features
- Branch: `minor` - Label:
![](https://via.placeholder.com/15/996633/000000?text=+) **[branch]
minor** - Content: Features affecting self-contained components such
as modules. Additions to Libraries, API, or modules that do not change
any function signatures.

#### Major Changes, Non Backwards-Compatible Changes and Large Features
- Branch: `major` - Label:
![](https://via.placeholder.com/15/4d3319/000000?text=+) **[branch]
major** - Content: Any change modifying a function signature in a
library class. Features require extensive LORIS-wide testing. New
complex systems and features spanning across multiple modules and
libraries. Deprecated functions clean-up.

For more information about making well-organized pull requests,
please read our in-depth Wiki page, ["Contributing to the
Code"](https://github.com/aces/Loris/wiki/Contributing-to-the-Code).

## Pull Request Title and Description

To make it easier for reiewers to locate pull requests with wich they have
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in reviewers. (missing v)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in which too. (missing h)

expertise, we request that new pull requests follow a few conventions.

#### Title

The title should begin with square brackets enclosing the name of the
module that you are changing followed by a brief description.

e.g. This is a well-formed title. > [Imaging Uploader / Server Process
Manager] Fix invalid upload state

If you are instead editing the `Core` libraries or the `tools` directory,
etc., you can supply these values in place of a module name.

There should be plenty of other examples in the list of pull requests
in the main code repository.

#### Description

When opening a pull request on GitHub you will see a pull request
template. Please fill out each heading with detailed information on your
code changes, suggested testing instructions, and links to open GitHub
issues and/or Redmine tickets (if applicable).

## Some Things To Keep In Mind

* If your changes require any table modifications:
1. Review our [SQL standard](./docs/SQLModelingStandard.md).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should "SQL standard" be "SQL standards"

If it is the case, the file should also be renamed. Who created that file? Oh wait, I created that file :-(.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think either one is fine. :)

2. Modify the `SQL/0000*.sql` file(s) with your changes. These patches
are applied during the LORIS install. 3. Include a patch to be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. and 4. should be at the same distance from beginning of line as 1. and 2. for rendering.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PapillonMcGill This is actually how it's intended to appear. 1. and 2. are sub instructions underneath the first * as they relate specifically to table modifications.

used by existing projects. These should go in the `SQL/New_patches/`
directory. 4. For SQL patches that are _optional_ (e.g. those that
perform some cleanup), place them in `SQL/Cleanup_patches/`.
* Include a test for any new module in the `modules/MODULENAME/test/`
directory. You can look at other modules for examples of how to
write tests.
* Add your new automated tests to TravisCI in the `.travis.yml`.
* Make sure you run PHP codesniffer using the standards file in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it be changed to incorporate the command to run phpcs? eg.

Make sure you run PHP codesniffer using the standards file in docs/LorisCS.xml by running vendor/bin/phpcs --standard=docs/LorisCS.xml <path_to_changed_files> before sending any pull request, otherwise the automated tests will fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or atleast a reference to

with the command `vendor/bin/phpcs --standard=docs/LorisCS.xml [file]` for any

docs/LorisCS.xml by running `vendor/bin/phpcs --standard=docs/LorisCS.xml
<path_to_changed_files>` before sending any pull request,
otherwise the automated tests will fail. * Try and make all changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* (2) should be at the beginning of the line for rendering.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnsaigle This is the same rendering problem as higher.

backwards-compatible with existing installations. * If you must change
something in a non-backwards-compatible way - or if it would affect
the data or custom code of a study - document this in your pull request
description and
tag it with ![](https://via.placeholder.com/15/d4c5f9/000000?text=+)
**Caveat for Existing Projects**. This helps us to document our
release notes.

If you're unsure about any of the above, feel free to ask us for
clarification via the mailing list.

## Getting Started

If you're looking for ideas for a way to contribute but don't know where
to begin, here are some ideas to get you started:

* You can browse some of our public
[Issues](https://github.com/aces/Loris/issues). Issues tagged with
![](https://via.placeholder.com/15/0e8a16/000000?text=+) **Beginner
Friendly** are good ones to tackle if you are new to LORIS development.
* You can run PHP CodeSniffer on modules that haven't had it run yet.
* You can help improve our documentation if you find any parts of it confusing or
lacking
* You can try and track down any bugs
* You can help improve our documentation if you find any parts of it
confusing or
lacking.
* You can track down bugs by browsing the application and reviewing the
PHP error log.