How to Contribute

How to Contribute#

License#

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):

  1. Make a fork of the SkyPortal repository

  2. Clone your fork, and add upstream (git@github.com:skyportal/skyportal) as a remote

  3. Create a new branch and add your feature

  4. 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.

Bug Reports#

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:

  1. A full description of your environment, including operating system, and Python version.

  2. A minimal way to reproduce the problem you see; these can be either a set of instructions, or a script.

Process guidelines#

Because many developers work on SkyPortal, and PRs sometimes come in at a rapid pace, we have guidelines to streamline review and development:

Code style#

We don’t like arguing about code style, and likely you don’t either. Therefore, we use code formatters: black for Python, and Prettier for JavaScript. Code is an art, and opinions differ of what looks good: we choose to spend our time writing correct, elegant code.

Testing#

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” ✅.

Reviews#

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).

Focused PRs#

In SkyPortal, we squash all PRs before merging them. Because of this, it is okay to merge the main 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 main)

  • part-2 (based on feature-part-1)

  • part-3 (based on feature-part-2)

etc. Turn 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-2 while part-1 is still under review, make a PR onto your own part-1 branch, and direct reviewers there.

Git tips#

  • git reflog shows you the history of branch switches

  • git reset origin/main && git add --all squashes your branch into a single commit

  • git add -p adds only certain changes inside a file