Opened 5 months ago

Last modified 2 weeks ago

#29746 assigned defect

Improve Tor best practices tracker

Reported by: asn Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: practracker, tech-debt, refactoring, easy, 041-deferred-20190530, network-team-roadmap-july, dgoulet-merge
Cc: nickm, teor Actual Points: .1
Parent ID: Points: 3
Reviewer: teor, ahf Sponsor: Sponsor31-can

Child Tickets

TicketStatusOwnerSummaryComponent
#30752closednickmPractracker: usability improvements from May retrospectiveCore Tor/Tor
#31175closednickmTeach practracker to apply separate rules for C headersCore Tor/Tor
#31176closednickmTeach practracker about .may_include filesCore Tor/Tor
#31202closednickmRefactor practracker's issue-listing code to return a generatorCore Tor/Tor
#31263closednickmMore tests for practrackerCore Tor/Tor
#31304closednickmRun practracker_tests.py as part of make checkCore Tor/Tor
#31309closednickmAdd envvar to pass options to practrackerCore Tor/Tor
#31311closednickmpractracker: do not include unwanted files in distributionCore Tor/Tor
#31338assignednickmPractracker --list-overbroad produces confusing output when there is an exceptionCore Tor/Tor
#31476needs_reviewnickmPractracker: document new featuresCore Tor/Tor
#31477assignednickmPractracker integration tsts for headers and includesCore Tor/Tor

Change History (19)

comment:1 Changed 5 months ago by asn

Description: modified (diff)

comment:2 Changed 5 months ago by nickm

Description: modified (diff)

Editing description to remove an item that was already fixed.

comment:3 Changed 3 months ago by nickm

Keywords: 041-deferred-20190530 added

Marking these tickets as deferred from 041.

comment:4 Changed 3 months ago by nickm

Milestone: Tor: 0.4.1.x-finalTor: 0.4.2.x-final

comment:5 Changed 6 weeks ago by nickm

Points: 3

comment:6 Changed 5 weeks ago by nickm

Actual Points: .1
Owner: set to nickm
Status: newaccepted

See my branch practracker_fixes with PR at https://github.com/torproject/tor/pull/1177 . It includes fixes for the leftover issues above, and for the issues in #30752.

comment:7 Changed 5 weeks ago by nickm

Status: acceptedneeds_review

comment:8 Changed 5 weeks ago by gaba

Cc: nickm teor added
Keywords: network-team-roadmap-july added

comment:9 Changed 4 weeks ago by dgoulet

Reviewer: catalyst

comment:10 Changed 4 weeks ago by catalyst

Reviewer: catalystteor

comment:11 Changed 3 weeks ago by nickm

The abovementioned branch now also contains fixes for #31202 (refactoring) and #31263 (integration tests).

comment:12 Changed 3 weeks ago by ahf

Reviewer: teorteor, ahf
Status: needs_reviewmerge_ready

Non-blocking question: Why do we want to allow \t in #include statements (from commit 86d3d310f529bc1e13ce1ca5013c674837c9856b) ?

I think the code looks good, it's well split up and nothing crazy is going on. Being able to temporarily disable it in our build system is something I've wanted for a while, so that feature is very welcomed!

I'm going to say good to it here.

comment:13 Changed 3 weeks ago by nickm

The rationale for allowing the tab is that c allows it too.

Is this merge_ready for the whole branch, or just the first few commits?

comment:14 Changed 3 weeks ago by ahf

I reviewed all the patches in PR 1177: https://github.com/torproject/tor/pull/1177 yes.

comment:15 Changed 3 weeks ago by teor

Status: merge_readyneeds_revision

I added comments about two blocking issues to the pull request.

I also opened #31304 so we add practracker_tests.py to make check some time soon.

comment:16 Changed 3 weeks ago by nickm

Keywords: dgoulet-merge added
Status: needs_revisionmerge_ready

Okay, I've addressed the issues you found. Time to merge once travis accepts it.

(edit: travis hasn't passed yet, whoops)

Last edited 3 weeks ago by nickm (previous) (diff)

comment:17 Changed 3 weeks ago by dgoulet

Merged! As requested by nickm, not closing just yet.

comment:18 Changed 3 weeks ago by nickm

Status: merge_readyassigned

Okay, marking this as still assigned (as a parent ticket), and closing the appropriate child tickets.

comment:19 Changed 2 weeks ago by teor

I opened #31338 for a --list-overbroad output bug.

Note: See TracTickets for help on using tickets.