-
Notifications
You must be signed in to change notification settings - Fork 173
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
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
554e935
Update CONTRIBUTING.md
da4b8fa
More changes
010dc7f
Add front-end guide links
22a2d03
Rida's feedback
4002c66
Add colours and better format branch section
a161cee
Rewording
358d524
Update CONTRIBUTING.md
e25550c
made link change
ridz1208 079d772
Update CONTRIBUTING.md
ridz1208 186370d
Function signatures typo
c84b26a
Review SQL standard
d4582ed
phpcs details
3be5eb7
Add PR naming/description conventions
d26e28c
Better line lengths
johnsaigle 1b9256e
Github -> GitHub
johnsaigle a5c70d6
Fix bad rendering of list
johnsaigle 3a9f466
Actually fix rendering
johnsaigle 5c91bff
Cecile's feedback
johnsaigle 544834c
Remove fun coloured boxes :(
johnsaigle 64f0c72
typos
johnsaigle File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,47 +1,116 @@ | ||
# 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). If you are doing | ||
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: **[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: **[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: **[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 reviewers to locate pull requests with which they have | ||
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). | ||
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 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 | ||
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 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 **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 **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. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 :-(.
There was a problem hiding this comment.
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. :)