20679702a3
The text I've written here documents the current process as of today. The only major change I've added today is the `miniooni-staging` branch, which previously wasn't published and only lived in the private machine I was using for building `miniooni`.
168 lines
7.6 KiB
Markdown
168 lines
7.6 KiB
Markdown
# Contributing to ooni/probe-cli
|
|
|
|
This is an open source project, and contributions are welcome! You are welcome
|
|
to open pull requests. An open pull request will be reviewed by a core
|
|
developer. The review may request you to apply changes. Once the assigned
|
|
reviewer is satisfied, they will merge the pull request.
|
|
|
|
## OONI Software Development Guidelines
|
|
|
|
Please, make sure you read [OONI Software Development Guidelines](
|
|
https://ooni.org/post/ooni-software-development-guidelines/). We try in
|
|
general to follow these guidelines when working on ooni/probe-cli. In
|
|
the unlikely care where those guidelines conflict with this document, this
|
|
document will take precedence.
|
|
|
|
## Opening issues
|
|
|
|
Please, before opening a new issue, check whether the issue or feature request
|
|
you want us to consider has not already been reported by someone else. The
|
|
issue tracker is at [github.com/ooni/probe/issues](https://github.com/ooni/probe/issues).
|
|
|
|
## PR requirements
|
|
|
|
Every pull request that introduces new functionality should feature
|
|
comprehensive test coverage. Any pull request that modifies existing
|
|
functionality should pass existing tests. What's more, any new pull
|
|
request that modifies existing functionality should not decrease the
|
|
existing code coverage.
|
|
|
|
Long-running tests should be skipped when running tests in short mode
|
|
using `go test -short`. We prefer internal testing to external
|
|
testing. We generally have a file called `foo_test.go` with tests
|
|
for every `foo.go` file. Sometimes we separate long running
|
|
integration tests in a `foo_integration_test.go` file.
|
|
|
|
If there is a top-level DESIGN.md document, make sure such document is
|
|
kept in sync with code changes you have applied.
|
|
|
|
Do not submit large PRs. A reviewer can best service your PR if the
|
|
code changes are around 200-600 lines. (It is okay to add more changes
|
|
afterwards, if the reviewer asks you to do more work; the key point
|
|
here is that the PR should be reasonably sized when the review starts.)
|
|
|
|
In this vein, we'd rather structure a complex issue as a sequence of
|
|
small PRs, than have a single large PR address it all.
|
|
|
|
As a general rule, a PR is reviewed by reading the whole diff. Let us
|
|
know if you want us to read each diff individually, if you think that's
|
|
functional to better understand your changes.
|
|
|
|
## Code style requirements
|
|
|
|
Please, use `go fmt`, `go vet`, and `golint` to check your code
|
|
contribution before submitting a pull request. Make sure your code
|
|
is documented. At the minimum document all the exported symbols.
|
|
|
|
Make sure you commit `go.mod` and `go.sum` changes. Make sure you
|
|
run `go mod tidy` to minimize such changes.
|
|
|
|
## Implementation requirements
|
|
|
|
- use `./internal/atomicx` rather than `atomic/sync`
|
|
|
|
- do not use `os/exec`, use `x/sys/execabs` or `./internal/shellx`
|
|
|
|
- use `./internal/fsx.OpenFile` when you need to open a file
|
|
|
|
- use `./internal/netxlite.ReadAllContext` instead of `io.ReadAll`
|
|
and `./internal/netxlite.CopyContext` instead of `io.Copy`
|
|
|
|
## Code testing requirements
|
|
|
|
Make sure all tests pass with `go test -race ./...` run from the
|
|
top-level directory of this repository.
|
|
|
|
## Writing a new OONI experiment
|
|
|
|
When you are implementing a new experiment (aka nettest), make sure
|
|
you have read the relevant spec from the [ooni/spec](
|
|
https://github.com/ooni/spec) repository. If the spec is missing,
|
|
please help the pull request reviewer to create it. If the spec is
|
|
not clear, please let us know during the review.
|
|
|
|
To get a sense of what we expect from an experiment, see the [internal/tutorial](
|
|
https://github.com/ooni/probe-cli/tree/master/internal/tutorial) tutorial
|
|
|
|
## Branching and releasing
|
|
|
|
The following diagram illustrates the overall branching and releasing
|
|
strategy loosely followed by the core team. If you are an external
|
|
contributor, you generally only care about the development part, which
|
|
is on the left-hand side of the diagram.
|
|
|
|
|
|
|
|
Development uses the `master` branch. When we need to implement a
|
|
feature or fix a bug, we branch off of the `master` branch. We squash
|
|
and merge to include a feature or fix branch back into `master`.
|
|
|
|
We periodically tag `-alpha` releases directly on `master`. The
|
|
semantics of such releases is that we reached a point where we have
|
|
features we would like to test using the `miniooni` research CLI
|
|
client. As part of these releases, we also update dependencies and
|
|
embedded assets. This process ensures that we perform better testing
|
|
of dependencies and assets as part of development.
|
|
|
|
The `master` branch and pull requests only run CI lightweight tests
|
|
that ensure the code still compiles, has good coverage, and we are
|
|
not introducing regressions in terms of the measurement engine.
|
|
|
|
To draft a release we branch off of `master` and create a `release/x.y`
|
|
branch where `x` is the major number and `y` is the minor number. For
|
|
release branches, we enable a very comprehensive set of tests that run
|
|
automatically with every commit. The purpose of a release branch is to
|
|
make sure all checks are green and hotfix bugs that we may discover
|
|
as part of more extensively testing a release candidate. Beta and stable
|
|
releases should occur on this branch. Subsequent patch releases should
|
|
also occur on this branch. We have one such branch for each `x.y`
|
|
release. If there are fixes on `master` that we want to backport, we
|
|
cherry-pick them into the release branch. Likewise, if we need to
|
|
forward port fixes, we cherry-pick them into `master`. When we backport,
|
|
the commit message should start with `[backport]`; when we forward
|
|
port, the commit message should start with `[forwardport]`.
|
|
|
|
When we branch off release `x.y` from `master`, we also need to bump
|
|
the `alpha` version used by `master`.
|
|
|
|
In addition we have `PRODUCT-staging` branches for each product. These
|
|
branches are described below:
|
|
|
|
- `miniooni-staging`: branch for building and publishing `miniooni`
|
|
binaries. This branch marges from the `master` branch when we tag
|
|
alpha releases. We use this strategy because `miniooni` is an alpha client.
|
|
We publish the resulting binaries in alpha releases.
|
|
|
|
- `mobile-staging`: branch for building and publishing iOS and
|
|
Android releases of the `pkg/oonimkall` library. This branch
|
|
merges from the release branches or the release tags. We use this
|
|
strategy because mobile releases are either beta quality or
|
|
production quality releases. We publish the resulting binaries
|
|
in beta quality or stable releases.
|
|
|
|
- `oohelperd-staging`: branch for building and publishing `oohelperd`
|
|
binaries. This branch merges from the release branches or the
|
|
release tags. We use this strategy because `oohelperd` releases
|
|
are either beta quality or production quality releases. We
|
|
publish the resulting binaries in OONI's internal Debian repository.
|
|
|
|
- `ooniprobe-staging`: branch for building and publising `ooniprobe`
|
|
binaries. This branch merges from the release branches or the
|
|
release tags. We use this strategy because `ooniprobe` releases
|
|
are either beta quality or production quality releases. We
|
|
publish the resulting binaries both in beta quality or stable
|
|
releases and in OONI's public Debian repository.
|
|
|
|
Builds proper happen either in this repository, using GitHub
|
|
actions, or using private build machines. Sometimes we need
|
|
private build machines to inject specific secrets into the
|
|
binary (e.g., we currently use this to inject OONI's Psiphon config).
|
|
|
|
Additionally, we currently publish as Github Actions artifacts
|
|
`miniooni` binaries for selected Linux architectures for
|
|
each commit that lands into the `master` branch. You will
|
|
find those binaries [in the results of each individual build](
|
|
https://github.com/ooni/probe-cli/actions/workflows/miniooni.yml)
|
|
until they are automatically removed by GitHub compatibly with
|
|
the build assets retention period (should be 90 days).
|