Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: Add Maintaining.md documentation #451

Closed
wants to merge 4 commits into from

Conversation

Labels
None yet
Projects
None yet
5 participants
@dgoulet-tor
Copy link
Contributor

@dgoulet-tor dgoulet-tor commented Oct 29, 2018

Closes #28225

Signed-off-by: David Goulet dgoulet@torproject.org

Closes #28225

Signed-off-by: David Goulet <dgoulet@torproject.org>
@coveralls
Copy link

@coveralls coveralls commented Oct 29, 2018

Pull Request Test Coverage Report for Build 2620

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.001%) to 62.03%

Files with Coverage Reduction New Missed Lines %
src/feature/hs/hs_common.c 1 83.28%
Totals Coverage Status
Change from base Build 2595: -0.001%
Covered Lines: 44078
Relevant Lines: 71059

💛 - Coveralls

# Maintaining Tor

This document details the roles and processes on maintaining the Tor code
base. We do have many policies in place and the maintainers are there as a
Copy link
Member

@gabelula gabelula Oct 29, 2018

can we have a link here to the policies?

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 29, 2018

They are kind of all in the git tree in doc/HACKING/:

  • CodingStandards.md and CodingStandardsRust.md
  • CodeStructure.md
  • HowToReview.md
  • WritingTests.md
  • GettingStarted.md

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 29, 2018

That last part was removed.


This document details the roles and processes on maintaining the Tor code
base. We do have many policies in place and the maintainers are there as a
last line of defense to enforce them.
Copy link
Contributor

@nmathewson nmathewson Oct 29, 2018

IMO this paragraph is a little iffy. I don't agree that the main role of the maintainer is to defend against bad code. We're trying to make a better Tor, but that happens not only by rejecting things that are plainly noncompliant. We have to encourage people to get better, and try to help every release of Tor to be better than the one before.

I think our job isn't rejecting; it's oversight and teaching.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 29, 2018

Not sure this paragraph mentions "bad code" or anything about rejecting? ... But it is rather to enforce our code standard policies which is the "oversight" there that you mention.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 29, 2018

Fixup commit to remove that last sentence: 633135f


These are the tasks of a subsystem maintainer:

1. Regurlarly go over `merge_ready` tickets tagged with the keyword of the
Copy link
Contributor

@nmathewson nmathewson Oct 29, 2018

I suggest "relevant to the related subsystem" instead of "tagged with the keyword" here.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 29, 2018

Fixup 5c4412c

related subsystems and for the current alpha or development (master)
Milestone.

2. A subsystem maintainer is also in charge of contributing to any design
Copy link
Contributor

@nmathewson nmathewson Oct 29, 2018

s/in charge of contributing/expected to contribute/

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Oct 29, 2018

Fixup 4bca82f

@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Oct 29, 2018

squashed and merged!

@nmathewson nmathewson closed this Oct 29, 2018
Copy link
Contributor

@teor2345 teor2345 left a comment

Looks good.
I made a few trivial suggestions that we can fix when we do the stable maintainer part.

base.

The first section describes who is the current Tor maintainer and what are the
responsabilities. Tor has one main single maintainer but does have many
Copy link
Contributor

@teor2345 teor2345 Oct 30, 2018

typo: responsibilities


Upstream merges are restricted to the alpha and master branches. Subsystem
maintainers should never push a patch into a stable branch which is the
responsability of the [stable branch maintainer](#stable-branches).
Copy link
Contributor

@teor2345 teor2345 Oct 30, 2018

Typo: responsibility

These are few important items to follow when merging code upstream:

1. To merge code upstream, the patch must have passed our CI (currently
github.com/torproject), have a corresponding ticket and reviewed by
Copy link
Contributor

@teor2345 teor2345 Oct 30, 2018

If you want the full link in markdown, it's [our CI on GitHub](https://github.com/torproject/tor)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment