# Contributing to Simple Bandwidth Scanner

Thank you for your interest in Simple Bandwidth Scanner (`sbws`).

Examples of contributions include:

- Bug reports, feature requests
- Code/documentation patches

## Bug reports or feature requests

- Check that it has not been already reported.

- Open a ticket in Tor Project Trac and assign the component to `Core Tor`/`sbws`.

## Code/documentation patches

The sbws canonical repository is https://gitweb.torproject.org/sbws.git, but we review patches using the Github canonical repository (https://github.com/torproject/sbws) Pull Requests (PR).

To know more about `sbws` code,

See also

- Developer/technical documentation
- `./docs/source/testing.rst` (or testing or Installing tests dependencies and running tests).
- `./docs/source/documenting.rst` (or documenting or Installing documentation dependendencies and building it).

The following are guidelines we aim to follow.

### Steps to create a PR

1. Create a ticket in Tor Project Trac (Open ticket)
2. Clone `sbws` via the Github web interface https://github.com/torproject/sbws
3. Clone the repository locally
4. Install `sbws` as explained in ./INSTALL.rst and ./TESTING.rst Use `pip install -e <>`
5. If needed install the documentation and build it as explained in ./DOCUMENTATION.rst
6. Create a new branch, named `ticketXXX`. Optionally, name it with a string explaining what it does, ie `ticketXXX_contributing`
7. Write code (Code style), tests, documentation, extra files (Extra required files), commit (Commits), etc.
8. Ensure tests pass (./TESTING.rst).
9. Push your branch to your repository. If you have an account in Travis, you can see whether it pass the tests in Github and in https://travis-ci.org/youruser/sbws/
10. Create a PR from your branch to https://github.com/torproject/sbws
11. Change the Trac ticket status to `needs_review`

### Code style

Follow the Zen of Python (PEP 20)

    >>> import this
    The Zen of Python, by Tim Peters

    Beautiful is better than ugly.
    Explicit is better than implicit.
    Simple is better than complex.
    Complex is better than complicated.
    Flat is better than nested.
    Sparse is better than dense.
    Readability counts.

Code should adhere to the PEP 8 guidelines. Before release 1.0.0, some guidelines have not been followed, such as the ordering the inputs (PEP 8#imports).

External link: Code Style

All functions, methods and classes should have PEP 0257 (except `__repr__` and `__str__`). Before release 1.0.0, some docstrigs do not have 3 double quotes `"""` (PEP 0257#id15).

External link: Documentation

New features should add a corresponding documentation in /docs.

An editor compatible with EditorConfig will help you to follow the general formatting code style.

Timestamps must be in UTC. It is prefered to use `datetime` objects or Unix timestamps. Timestamps read by the user should be always formatted in ISO 8601

Functional style is prefered:

- use list comprenhensions lambda, map, reduce
- avoid reasigigning variables, instead create new ones
- use `deepcopy` when passing list of objects to a function/method
- classes should change attributes only in one method (other than __init__?)

[FUNC]

In general, do not reinvent the wheel, use Python native modules as `logging`, instead of implementing similar functionality. Or use other packages when the new dependency can be extra, for instance vulture.

### Extra required files

Any non-trivial change should contain tests. See ./TESTING.rst. When running tests, currently `flake8` informs on some PEP8 errors/warnings, but not all.

Document your changes in ./CHANGELOG.rst following keep a changelog. Reference the Tor Project Trac ticket (example: `#12345`) or Github ticket (example: `GH#123`).

### Commits

Try to make each commit a logically separate changes.:

    As a general rule, your messages should start with a single line that’s
    o more than about 50 characters and that describes the changeset concisely,
    followed by a blank line, followed by a more detailed explanation.
    The Git project requires that the more detailed explanation include
    your motivation for the change and contrast its implementation with
    previous behavior — this is a good guideline to follow.
    It’s also a good idea to use the imperative present tense in these messages.
    In other words, use commands.
    Instead of "I added tests for" or "Adding tests for," use "Add tests for."

[DIST]

Template originally written by Tim Pope: example commit

### Code being reviewed workflow

When a PR is being reviewed, new changes might be needed:

- If the change does not modify a previous change, create new commits and push.

- If the change modifies a previous change and it’s small, git commit fixup should be used. When it is agreed that the PR is ready, create a new branch named `mybranch_02` and run:

      rebase --autosquash

  push, create new PR and close old PR mentioning the number of the new PR.

- If the review takes long and when it’s ready code related to the PR has changed in master, create a new branch named `mybranch_02` and run:

      rebase master

  push, create new PR and close old PR mentioning the number of the new PR.

[MERG]

## Reviewing code

All code should be peer-reviewed. Two reasons for this are:

    Because a developer cannot think of everything at once;
    Because a fresh pair of eyes may spot an error, a corner-case in the code,
    insufficient documentation, a missing consistency check, etc.

[REVI]

Reviewers:

- Should let the contributor know what to improve/change.
- Should not push code to the contributor’s branch.
- Should wait for contributor’s changes or feedback after changes are requested, before merging or closing a PR.
- Should merge (not rebase) the PR.
- If rebase is needed due to changes in master, the contributor should create a new branch named xxx_rebased based on the reviewed branch, rebase and create a new PR from it, as explained above.
- If new changes are needed when the contributor’s branch is ready to merge, the reviewer can create a new branch based on the contributor’s branch, push the changes and merge that PR. The contributor should be notified about it.
- If the reviewer realize that new changes are needed after the PR has been merged, the reviewer can push to master, notifying the contributor about the changes.
- Because currently there are not many reviewers, reviewers can merge their own PR if there was not any feedback after a week.
- Should not push directly to master, unless changes are trivial (typos, extra spaces, etc.)
- Should not push to master new features while there are open PRs to review.

Currently, the reviewers are the persons that have contributed to the code: pastly, teor, juga.

## Releases

Releases follow semantic versioning. Until release 1.0.0 is reached, this project is not considered production ready.

Currently development happens in master, this might change from release 1.0.0

so that master has the last release changes, and development happens in the next release branch.

Before major releases, ensure that:

- Installation from scratch, as specified in ./INSTALL.md, must success.

- All tests must pass.

- Tor must be able to parse the produced bw files (current way is manual)

  Todo

  Test that run Tor as dirauth and parse the files

- Bandwidth files must produce graphs compatible with Torflow (current way to test it is manual)

  Todo

  Implement something to compare error with current consensus.

- A dirauth should be able to understand the documentation, otherwise the documentation should be clarified.

## Example commit message

    Short (50 chars or less) summary of changes

    More detailed explanatory text, if necessary.  Wrap it to
    about 72 characters or so.  In some contexts, the first
    line is treated as the subject of an email and the rest of
    the text as the body.  The blank line separating the
    summary from the body is critical (unless you omit the body
    entirely); tools like rebase can get confused if you run
    the two together.

    Further paragraphs come after blank lines.

      - Bullet points are okay, too

      - Typically a hyphen or asterisk is used for the bullet,
        preceded by a single space, with blank lines in
        between, but conventions vary here

External eferences

[DIST] | https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project

[MERG] | https://www.atlassian.com/git/tutorials/merging-vs-rebasing

[REVI] | https://doc.sagemath.org/html/en/developer/reviewer_checklist.html

[FUNC] | https://medium.com/@rohanrony/functional-programming-in-python-1-lambda-map-filter-reduce-zip-8739ea144186