[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [sbws/master] Move sections
commit b47d4bef41a2ac271fb31d43c63ba6abacce64dc
Author: juga0 <juga@xxxxxxxxxx>
Date: Tue Sep 18 15:00:18 2018 +0000
Move sections
---
CONTRIBUTING.rst | 194 ++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 158 insertions(+), 36 deletions(-)
diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index b35de63..186169b 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -87,81 +87,203 @@ 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`).
-Any non-trivial change should contain tests. See ./TESTING.rst.
-When running tests, currently ``flake8`` informs on some PEP8 errors/warnings,
-but not all.
+External link: `Code Style <https://docs.python-guide.org/writing/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`).
-New features should add a corresponding documentation.
+External link: `Documentation <https://docs.python-guide.org/writing/documentation/>`_
-Document your changes in ./CHANGELOG.rst following `keep a changelog`_.
-Reference the Tor Project Trac ticket (example: ``#12345``) or
-Github ticket (example: ``GH#123``).
+New features should add a corresponding documentation in /docs.
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 <https://en.wikipedia.org/wiki/ISO_8601>`_
-Git workflow
-------------
+Functional style is prefered:
-Commits
-~~~~~~~~
+- 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__?)
-Commit messages should follow the `Tim Pope`_ recommendations.
+[FUNC]_
-Workflow
+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`_.
+
+.. _`extrafiles-ref`:
+
+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-ref:
+
+Commits
~~~~~~~~~
-In general, when you are modifying code that was not wrotten by yourself,
-try to keep changes to the minimum.
+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`_: :ref:`example commit <commit-msg>`
+
+Code being reviewed workflow
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
When a PR is being reviewed, new changes might be needed:
-- If the change does not modify a previous change, just commit and push.
+- 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 <https://git-scm.com/docs/git-commit#git-commit---fixupltcommitgt>`_
should be used. When it is agreed that the PR is ready, create a new branch
- named ``mybranch_02`` and run::
+ named ``mybranch_02`` and run:
+
+ .. code-block:: bash
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::
+ in master, create a new branch named ``mybranch_02`` and run:
+
+ .. code-block:: bash
rebase master
push, create new PR and close old PR mentioning the number of the new PR.
-Reviewers: (see :ref:`reviewers`)
+[MERG]_
-- should not push code to your branch, unless you agree
-- should let you know when new changes are needed
-- should not merge your PR after changes are requested and you notify you did
- via the PR or the ticket.
-- should not merge your PR and then inmediatly modify your code pushing
- directly to master without informing you previously and without your consent.
+.. _review-ref:
-.. _reviewers:
+Reviewing code
+----------------
-Reviewers
+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-ref:
+
+Releases
----------
-At the moment, there is not any policy to decide who the reviewers are.
-They are the current people that has contributed to this code: pastly, teor,
-juga0.
-They should not push directly to master and they should peer-review their code.
-Currently, if a PR from a reviewer has not been peer-reviewd by other reviewer
-in a week, the reviewer can merge their/her/his own PR.
+Releases follow `semantic versioning`_.
+Until release 1.0.0 is reached, this project is not considered production
+ready.
-They should merge PR to master. Instead of rebase. If needed, rebase should be
-done by the contributor before the merge.
+Currently development happens in master, this might change from release 1.0.0
-.. _tim pope: https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
+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.
+
+
+.. _commit-msg:
+
+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
+
+
+.. rubric:: 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
+.. _tim pope: https://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html
.. _`keep a changelog`: https://keepachangelog.com/en/1.0.0/
+.. _`semantic versioning`: https://semver.org/
+.. _`vulture`: https://pypi.org/project/vulture/
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits