ooni-probe-cli/CONTRIBUTING.md
Simone Basso 2e0118d1a6
refactor(netxlite): hide details without breaking the rest of the tree (#454)
## 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
2021-09-05 14:49:38 +02:00

3.8 KiB

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

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 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 tutorial

Thank you!