developing: add development guide for new comers
This change introduces a DEVELOPING.md file, commonly seen across open source repositories as a way to communicate how to get started contributing code to new-comers. Change-Id: Idb92231e025250a4c6d2fc789cab5f78ca87086a
This commit is contained in:
parent
985ccbe721
commit
08b87218bf
389
DEVELOPING.md
Normal file
389
DEVELOPING.md
Normal file
@ -0,0 +1,389 @@
|
||||
# Purpose
|
||||
|
||||
To provide useful information for developers and maintainers of Storj.
|
||||
|
||||
# Tooling
|
||||
|
||||
## Development
|
||||
|
||||
[`storj-up`](https://github.com/storj/up) provides a convenient way to configure and spin up a cluster locally for
|
||||
active development. In addition to `storj-up`, you will need the following tools:
|
||||
|
||||
- TODO
|
||||
|
||||
## Version Control and Code Review
|
||||
|
||||
All of our source code is hosted on [GitHub](https://github.com/storj) and most of our reviews are done in
|
||||
[Gerrit](https://review.dev.storj.io). Reviews require 2 `Code Review +2` and a passing build in order to be merged.
|
||||
|
||||
## Continuous Integration
|
||||
|
||||
Currently, builds are performed by a [Jenkins](https://build.dev.storj.io) cluster hosted in Google Cloud. Because of
|
||||
issues with how Google Cloud connects their disk to your virtual machine, we've disabled debug logging in our tests for
|
||||
the time being. If a build fails and the failure message is insufficient for troubleshooting the test failure, rerunning
|
||||
the test locally will output debug logs to aid in the troubleshooting.
|
||||
|
||||
# Practices
|
||||
|
||||
## Git Workflow
|
||||
|
||||
Storj uses a [Gerrit][gerrit-link] based git workflow. While pull requests can be made against the public GitHub
|
||||
repository, many engineers at Storj prefer Gerrit for code reviews. For an overview of how this works, you can read
|
||||
the [Intro to Gerrit Walkthrough][gerrit-walkthrough-link]. Be sure to [sign in][] to Gerrit before attempting to
|
||||
contribute any changes. You'll likely need to upload an SSH key in order to push any changes.
|
||||
|
||||
[sign in]: https://review.dev.storj.io
|
||||
[gerrit-link]: https://review.dev.storj.io/Documentation/index.html
|
||||
[gerrit-walkthrough-link]: https://review.dev.storj.io/Documentation/intro-gerrit-walkthrough-github.html
|
||||
|
||||
Below, you'll find several common workflows that I've used when contributing code.
|
||||
|
||||
### Cloning a project
|
||||
|
||||
When a project uses Gerrit, there is some additional work that needs to be done before you can start contributing.
|
||||
|
||||
1. Clone the repository.
|
||||
```shell
|
||||
git clone git@github.com:storj/storj.git && cd storj
|
||||
```
|
||||
2. Setup Gerrit commit hook and aliases.
|
||||
```shell
|
||||
curl -L storj.io/clone | sh
|
||||
```
|
||||
|
||||
That should be it. At this point, the repository should be setup properly for Gerrit.
|
||||
|
||||
### Starting a new change set
|
||||
|
||||
When starting a new change, I find it useful to start a branch from the latest main (just like you would with any other
|
||||
GitHub project). Unlike GitHub, we will not be pushing this branch remotely.
|
||||
|
||||
```shell
|
||||
git checkout main
|
||||
git pull
|
||||
git checkout -b branch-name
|
||||
```
|
||||
|
||||
This is where you'll work on and commit your code. When you commit your changes, your commit message should not only
|
||||
describe what is changing, but why the change is being made. Storj uses the following convention for commit messages:
|
||||
|
||||
```
|
||||
{scope}: {message}
|
||||
|
||||
{detail}
|
||||
|
||||
Change-Id: {changeID}
|
||||
```
|
||||
|
||||
- `{scope}` refers to path within the repo that's changing. For example `satellite/metainfo` or `web/storagenode`.
|
||||
Multiple scopes _can_ be provided, but should be minimized.
|
||||
- `{message}` should provide a clear and succinct message about what is changing. Using words like `add`, `remove`,
|
||||
`reuse`, or `refactor` are great here.
|
||||
- `{detail}` provides additional information about the change. This is a good place to example why the change is being
|
||||
made. If you're making performance related changes, then this should also include a performance report comparing the
|
||||
before and after ([example][performance-example-link]).
|
||||
- `{changeID}` refers to the automatically generated change id.
|
||||
|
||||
[performance-example-link]: https://github.com/storj/picobuf/commit/1d3412eb3ac13a476e56fa0e732552ed2ee89ecf
|
||||
|
||||
To produce this commit message, I find it easiest to start with the scope and message and then amend the commit to add
|
||||
the detail. This allows the commit hook to automatically add the change id before filling in the longer detail of the
|
||||
commit message. In addition to that, amending the commit provides some text guides that can be a useful reference when
|
||||
determining where to wrap your detail text.
|
||||
|
||||
```shell
|
||||
git commit -m "{scope}: {message}"
|
||||
git commit --amend
|
||||
```
|
||||
|
||||
Once a change has been commit, you can use `git push-wip` to push a work-in-progress change or `git push-review` to push
|
||||
a new review. A work-in-progress change will allow you to push code without running tests or notifying the team of a new
|
||||
review. All active reviews will get pulled into our review notification system and run our CI pipeline.
|
||||
|
||||
### Updating an existing change set
|
||||
|
||||
Updating an existing change set is a fairly common task. This can happen while iterating on a change or even updating it
|
||||
with the latest set of changes from main. In the former case (updating while iterating on a change), you simply need to
|
||||
amend your existing commit with your latest changes.
|
||||
|
||||
```shell
|
||||
git add <updates>
|
||||
git commit --amend --no-edit
|
||||
```
|
||||
|
||||
The `--amend` flag tells git that you want the current change set to be added to the last git commit on your current
|
||||
branch. The `--no-edit` flag instructs git to leave the commit message as is, and not to prompt the end user for
|
||||
modifications to the message.
|
||||
|
||||
In the later case (when updating with the latest set of changes from main), you simply need to rebase your branch on the
|
||||
updates from main.
|
||||
|
||||
```shell
|
||||
git checkout main
|
||||
git pull
|
||||
git checkout branch-name
|
||||
git rebase main
|
||||
```
|
||||
|
||||
After any kind of change, you simply need to `push-wip` or `push-review` again.
|
||||
|
||||
### Managing relation chains
|
||||
|
||||
In Gerrit, multiple changes can be linked together for larger features. This is done by adding multiple commits to the
|
||||
same branch. Consider the `oidc/implementation` branch below.
|
||||
|
||||
```
|
||||
commit 533b6a2624f2a2eab45f0d68a6dd6e4fd1ec1124 (HEAD -> oidc/implementation)
|
||||
Author: Mya <redacted>
|
||||
Date: Mon Feb 14 14:06:35 2022 -0600
|
||||
|
||||
web/satellite: add consent screen for oauth
|
||||
|
||||
When an application wants to interact with resources on behalf of
|
||||
an end-user, it needs to be granted access. In OAuth, this is done
|
||||
when a user submits the consent screen.
|
||||
|
||||
Change-Id: Id838772f76999f63f5c9dbdda0995697b41c123a
|
||||
|
||||
commit ed9b75b11ecde87649621f6a02d127408828152e
|
||||
Author: Mya <redacted>
|
||||
Date: Tue Feb 8 15:28:11 2022 -0600
|
||||
|
||||
satellite/oidc: add integration test
|
||||
|
||||
This change adds an integration test that performs an OAuth
|
||||
workflow and verifies the OIDC endpoints are functioning as
|
||||
expected.
|
||||
|
||||
Change-Id: I18a8968b4f0385a1e4de6784dee68e1b51df86f7
|
||||
|
||||
commit 187aa654ff18387d0011b3741cca62270b95d6e6
|
||||
Author: Mya <redacted>
|
||||
Date: Thu Feb 3 14:49:38 2022 -0600
|
||||
|
||||
satellite/console: added oidc endpoints
|
||||
|
||||
This change adds endpoints for supporting OpenID Connect (OIDC) and
|
||||
OAuth requests. This allows application developers to easily
|
||||
develop apps with Storj using common mechanisms for authentication
|
||||
and authorization.
|
||||
|
||||
Change-Id: I2a76d48bd1241367aa2d1e3309f6f65d6d6ea4dc
|
||||
|
||||
```
|
||||
|
||||
These commits result in 3 distinct Gerrit changes, each linked together. If the last change in the link is merged, then
|
||||
all other changes are merged along with it.
|
||||
|
||||
#### Editing or dropping a commit
|
||||
|
||||
In many cases, we need to go back and amend a previous change because of some feedback, a failing test, etc. If you're
|
||||
editing an existing change or dropping and old one then you can use an interactive rebase to `edit` or `drop` a previous
|
||||
commit. The command below will start an interactive rebase with the 3 most recent commits. (You should adjust this
|
||||
number as needed.)
|
||||
|
||||
```shell
|
||||
git rebase -i HEAD~3
|
||||
```
|
||||
|
||||
This should bring up a screen where you can choose which commits you want to `edit` and which ones you want to `drop`.
|
||||
The code block below shows how this screen looks for the associated changes above.
|
||||
|
||||
```
|
||||
pick 187aa654f satellite/console: added oidc endpoints
|
||||
pick ed9b75b11 satellite/oidc: add integration test
|
||||
pick 533b6a262 web/satellite: add consent screen for oauth
|
||||
|
||||
# Rebase d4cf2013e..533b6a262 onto d4cf2013e (3 commands)
|
||||
#
|
||||
# Commands:
|
||||
# p, pick <commit> = use commit
|
||||
# r, reword <commit> = use commit, but edit the commit message
|
||||
# e, edit <commit> = use commit, but stop for amending
|
||||
# s, squash <commit> = use commit, but meld into previous commit
|
||||
# f, fixup [-C | -c] <commit> = like "squash" but keep only the previous
|
||||
# commit's log message, unless -C is used, in which case
|
||||
# keep only this commit's message; -c is same as -C but
|
||||
# opens the editor
|
||||
# x, exec <command> = run command (the rest of the line) using shell
|
||||
# b, break = stop here (continue rebase later with 'git rebase --continue')
|
||||
# d, drop <commit> = remove commit
|
||||
# l, label <label> = label current HEAD with a name
|
||||
# t, reset <label> = reset HEAD to a label
|
||||
# m, merge [-C <commit> | -c <commit>] <label> [# <oneline>]
|
||||
# . create a merge commit using the original merge commit's
|
||||
# . message (or the oneline, if no original merge commit was
|
||||
# . specified); use -c <commit> to reword the commit message
|
||||
#
|
||||
# These lines can be re-ordered; they are executed from top to bottom.
|
||||
#
|
||||
# If you remove a line here THAT COMMIT WILL BE LOST.
|
||||
#
|
||||
# However, if you remove everything, the rebase will be aborted.
|
||||
#
|
||||
```
|
||||
|
||||
#### Adding a new commit
|
||||
|
||||
To add a new commit to the relation chain, `edit` the commit just before where you want to add one, then make or unstash
|
||||
your modifications, and commit as you would [above](#starting-a-new-change-set). Continuing the rebase will re-apply
|
||||
your later changes on top of your newly created commit.
|
||||
|
||||
#### Splitting a commit
|
||||
|
||||
Splitting an existing change into multiple changes can be significantly more difficult. You need to consider not only
|
||||
how a change needs to be split, but the order in which the commits must be applied. In this case, it's preferred that
|
||||
one of the split commits preserve the existing change id as it's likely your change has already accumulated some
|
||||
feedback that we want to preserve. When splitting a commit, I prefer to create a new branch before rebasing the change.
|
||||
|
||||
```shell
|
||||
git checkout oidc/implementation
|
||||
git checkout -b oidc/rework
|
||||
|
||||
git rebase -i HEAD~3
|
||||
|
||||
...
|
||||
```
|
||||
|
||||
This way, if I make a mistake or need to start over, I can go back to a known working version by aborting the rebase,
|
||||
going back to the old branch, and deleting the rework.
|
||||
|
||||
```shell
|
||||
git rebase --abort
|
||||
git checkout oidc/implementation
|
||||
git branch -D oidc/rework
|
||||
|
||||
git branch -b oidc/rework
|
||||
|
||||
git rebase -i HEAD~3
|
||||
|
||||
...
|
||||
```
|
||||
|
||||
## Developing locally with `storj-up`
|
||||
|
||||
Following the instructions in the `storj-up` project `README`, the following will deploy a copy of the stack.
|
||||
|
||||
```shell
|
||||
storj-up init
|
||||
docker compose up -d
|
||||
```
|
||||
|
||||
Outside the automation `storj-up` provides, there are a handful of manual changes that can be made to support testing
|
||||
and developing against different portions of the stack. In the following sections, we demonstrate how this can be done
|
||||
using the satellite process as an example, but the same process should work with many of our other processes.
|
||||
|
||||
### Testing backend changes
|
||||
|
||||
To test local backend changes, all you need to do is tell `storj-up` to mount a local binary for those containers.
|
||||
Before mounting the local binary, you'll need to ensure your local binary is up-to-date.
|
||||
|
||||
```shell
|
||||
# on Linux
|
||||
go install ./cmd/satellite
|
||||
storj-up local-bin satellite-core satellite-admin satellite-api
|
||||
|
||||
# on OSX
|
||||
GOOS=linux GOARCH=amd64 go install ./cmd/satellite
|
||||
storj-up local-bin -s linux_amd64 satellite-core satellite-admin satellite-api
|
||||
```
|
||||
|
||||
Once `storj-up` completes, you'll need to redeploy the containers to pick up the new configuration.
|
||||
|
||||
```shell
|
||||
docker compose up -d
|
||||
```
|
||||
|
||||
From here on out, all you'll need to do is recompile the binary and restart the container to pick up your latest
|
||||
changes.
|
||||
|
||||
```shell
|
||||
# on Linux
|
||||
go install ./cmd/satellite
|
||||
docker restart storj-satellite-core-1 storj-satellite-api-1 storj-satellite-admin-1
|
||||
|
||||
# on OSX
|
||||
GOOS=linux GOARCH=amd64 go install ./cmd/satellite
|
||||
docker restart storj-satellite-core-1 storj-satellite-api-1 storj-satellite-admin-1
|
||||
```
|
||||
|
||||
_Aside_ - A few more notes here on OSX:
|
||||
|
||||
- Ensure your `GOPATH` is mountable by Docker. On OSX, only certain folders are mounted into the VM so you'll need to
|
||||
make sure it falls under one of those. Otherwise, `storj-up` will attempt to mount a directory under `GOBIN` that is
|
||||
unavailable on the VM.
|
||||
- Even if you're on a Mac M1 (like me), you should target an `amd64` architecture. OSX provides some supporting tools
|
||||
that allows you to run `amd64` workloads on `arm64` processors.
|
||||
|
||||
### Testing frontend changes
|
||||
|
||||
In order to support mounting local web builds into the container, manual changes needed to be made to the
|
||||
`satellite-api` volumes block of the `docker-compose.yaml` file. The following block adds a bind mount for the web
|
||||
assets. Be sure to replace `${SOURCE_DIR}` with the path to the storj source repository (`pwd` on OSX and Linux).
|
||||
|
||||
```yaml
|
||||
- type: bind
|
||||
source: ${SOURCE_DIR}/web/satellite/dist/
|
||||
target: /var/lib/storj/storj/web/satellite/dist/
|
||||
bind:
|
||||
create_host_path: true
|
||||
```
|
||||
|
||||
You’ll need to restart the container to pick up the new volume.
|
||||
|
||||
```shell
|
||||
docker compose up -d
|
||||
```
|
||||
|
||||
Now, you just need to rebuild the web assets, and they’ll automatically be picked up by the `satellite-api` deployment.
|
||||
|
||||
```shell
|
||||
cd web/satellite
|
||||
|
||||
npm run build
|
||||
```
|
||||
|
||||
#### Testing changes to static files (including wasm)
|
||||
|
||||
Any changes to the `static` directory (including wasm) will require an additional bind mount for the `satellite-api`.
|
||||
|
||||
```yaml
|
||||
- type: bind
|
||||
source: ${SOURCE_DIR}/web/satellite/static/
|
||||
target: /var/lib/storj/storj/web/satellite/static/
|
||||
bind:
|
||||
create_host_path: true
|
||||
```
|
||||
|
||||
Similarly, you’ll need to restart the container to pick up the new volume.
|
||||
|
||||
```shell
|
||||
docker compose up -d
|
||||
```
|
||||
|
||||
Now, any changes to the `static` directory will automatically be picked up by the backend. If you’re iterating on the
|
||||
wasm module, you can invoke the `wasm` or `wasm-dev` npm targets.
|
||||
|
||||
```shell
|
||||
cd web/satellite
|
||||
|
||||
npm run wasm
|
||||
```
|
||||
|
||||
# Specific Contribution Workflows
|
||||
|
||||
<!-- translations? -->
|
||||
|
||||
## Security patches
|
||||
|
||||
If you're submitting a patch related to a core vulnerability of the product, the review should be submit as a
|
||||
work-in-progress, marked private, and the following users should be added.
|
||||
|
||||
- JT Olio
|
||||
- Kaloyan Raev
|
||||
- Egon Elibre
|
||||
- Jeff Wendling
|
||||
- Márton Elek
|
||||
- Mya Pitzeruse
|
||||
|
||||
Together, we'll work through how to best roll the changes out to our network.
|
Loading…
Reference in New Issue
Block a user