How to Contribute¶
SkyPortal is released under the Modified BSD license, which means that you are allowed to modify the code for your own purposes, as long as you retain our copyright notice.
However, we would love to grow the SkyPortal community, and integrate improvements directly into our code repository on GitHub.
Including your changes¶
To make a code contribution to the project, follow these steps (which are outlined in more detail in this GitHub guide):
Make a fork of the SkyPortal repository
Clone your fork, and add
email@example.com:skyportal/skyportal) as a remote
Create a new branch and add your feature
Submit a pull request (PR) on GitHub
The other developers will provide feedback, and you may push updates into the same branch (which will also update your pull request), until the Continuous Integration tests pass and reviewers agree that it should be merged (see “Process Guidelines: Reviews” below).
For a more detailed explanation of the open contribution process, see the scikit-image contributors’ guide. We follow a very similar process; some guidance follows below.
While we appreciate code changes, it is also very helpful simply to know when SkyPortal does not function correctly. Please file any issues you run across.
If possible, provide:
A full description of your environment, including operating system, and Python version.
A minimal way to reproduce the problem you see; these can be either a set of instructions, or a script.
Because many developers work on SkyPortal, and PRs sometimes come in at a rapid pace, we have guidelines to streamline review and development:
All functionality should be accompanied by tests. We use pytest and Selenium with Geckodriver. PRs can only be merged once tests have been added and pass. The continuous integration system indicates this with a green checkmark, hence you may see developers talking about “PR 599 being green” ✅.
All code that goes into SkyPortal is reviewed by two team members (team members are persons with commit access) or, if the contributor is a team member, by at least one other team member. We find review invaluable for improving code quality, and no author ever merges their own work.
We ask all reviewers to be mindful that there is a human at the other side of the PR. As such, consider the review process as a conversation aimed at getting the work mergeable as quickly as possible:
Do not nitpick in comments; add small fixes as suggestions (click the
±button) or push uncontroversial changes as a commit into the PR branch. We prefer suggestions, because that stil gives the original author a chance to accept/reject the change.
Comments should provide provide actionable feedback, and can come in two flavors: suggestions and requirements. Requirements are direct instructions such as “Define the variable first”, whereas suggestions are softer: “Consider whether you’d like to reformat this dictionary” or “Recommendation: prefer the shorthand form”.
When comments are left on your PR, let the reviewer resolve their comments once they are satisfied they’ve been adequately addressed (unless you’ve done exactly as they asked).
In SkyPortal, we squash all PRs before merging them. Because of this, it is okay to merge the master branch into your branch, instead of rebasing.
The resulting squashed commit should deal with one topic only. For example, if you find that, while implementing a new component, you also need to fix another, split that out into a separate PR.
Long PRs take much longer to review than shorter ones. So, for the benefit of the developer and reviewers alike, we ask that PRs are kept as small as possible.
Describe your work¶
When a PR is squashed, all commit messages are listed. Please provide information in commit messages on what they do—this helps us track bugs down later. A commit message has a title and a description:
Return JSON when accessing invalid API endpoint (#644) Currently, the main page gets rendered for invalid API endpoint requests (i.e., `/api/*` where `*` is invalid). Now, it returns 400 as JSON with an error message in the `message` field.
The description may be ommitted for small fixes.
Remember that you can augment the last commit using
git commit --amend, instead of making numerous
Also fixed x type commits.
Very large, dependent sets of changes¶
Sometimes, you are working on a feature that is large enough that it spans several sequential PRs. In this case, each branch will depend on the previous, so you may have:
part-1(based on master)
part-2(based on feature-part-1)
part-3(based on feature-part-2)
part-1 into a PR and, once it is merged, move on to
part-2. If you would also like others to look at
part-1 is still under review, make a PR onto your own
branch, and direct reviewers there.
git reflogshows you the history of branch switches
git reset origin/master && git add --allsquashes your branch into a single commit
git add -padds only certain changes inside a file