2e0118d1a6
## Description This PR continues the refactoring of `netx` under the following principles: 1. do not break the rest of the tree and do not engage in extensive tree-wide refactoring yet 2. move under `netxlite` clearly related subpackages (e.g., `iox`, `netxmocks`) 3. move into `internal/netxlite/internal` stuff that is clearly private of `netxlite` 4. hide implementation details in `netxlite` pending new factories 5. refactor `tls` code in `netxlite` to clearly separate `crypto/tls` code from `utls` code After each commit, I run `go test -short -race ./...` locally. Each individual commit explains what it does. I will squash, but this operation will preserve the original commit titles, so this will give further insight on each step. ## Commits * refactor: rename netxmocks -> netxlite/mocks Part of https://github.com/ooni/probe/issues/1591 * refactor: rename quicx -> netxlite/quicx See https://github.com/ooni/probe/issues/1591 * refactor: rename iox -> netxlite/iox Regenerate sources and make sure the tests pass. See https://github.com/ooni/probe/issues/1591. * refactor(iox): move MockableReader to netxlite/mocks See https://github.com/ooni/probe/issues/1591 * refactor(netxlite): generator is an implementation detail See https://github.com/ooni/probe/issues/1591 * refactor(netxlite): separate tls and utls code See https://github.com/ooni/probe/issues/1591 * refactor(netxlite): hide most types but keep old names as legacy With this change we avoid breaking the rest of the tree, but we start hiding some implementation details a bit. Factories will follow. See https://github.com/ooni/probe/issues/1591
97 lines
3.8 KiB
Markdown
97 lines
3.8 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/iox.ReadAllContext` instead of `io.ReadAll`
|
|
and `./internal/netxlite/iox.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.
|
|
|
|
When you write a new experiment, keep the measurement phase and the
|
|
results analysis phases as separate functions. This helps us a lot
|
|
to write better unit tests for our code.
|
|
|
|
To get a sense of what we expect from an experiment, see:
|
|
|
|
- the internal/engine/experiment/example experiment
|
|
|
|
- the internal/engine/experiment/webconnectivity experiment
|
|
|
|
- the [internal/tutorial](https://github.com/ooni/probe-cli/tree/master/internal/tutorial) tutorial
|
|
|
|
Thank you!
|