Opened 7 months ago

Closed 5 days ago

#29746 closed enhancement (fixed)

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 042-can
Cc: nickm, teor Actual Points: .1
Parent ID: Points: 3
Reviewer: teor, ahf Sponsor: Sponsor31-can

Child Tickets

TicketTypeStatusOwnerSummary
#30752enhancementclosednickmPractracker: usability improvements from May retrospective
#31175enhancementclosednickmTeach practracker to apply separate rules for C headers
#31176enhancementclosednickmTeach practracker about .may_include files
#31202defectclosednickmRefactor practracker's issue-listing code to return a generator
#31263enhancementclosednickmMore tests for practracker
#31304enhancementclosednickmRun practracker_tests.py as part of make check
#31309defectclosednickmAdd envvar to pass options to practracker
#31311defectclosednickmpractracker: do not include unwanted files in distribution
#31338defectclosednickmPractracker --list-overbroad produces confusing output when there is an exception
#31476taskclosednickmPractracker: document new features
#31477defectclosednickmPractracker integration tests for headers and includes

Change History (23)

comment:1 Changed 7 months ago by asn

Description: modified (diff)

comment:2 Changed 7 months ago by nickm

Description: modified (diff)

Editing description to remove an item that was already fixed.

comment:3 Changed 5 months ago by nickm

Keywords: 041-deferred-20190530 added

Marking these tickets as deferred from 041.

comment:4 Changed 5 months ago by nickm

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

comment:5 Changed 3 months ago by nickm

Points: 3

comment:6 Changed 3 months 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 3 months ago by nickm

Status: acceptedneeds_review

comment:8 Changed 3 months ago by gaba

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

comment:9 Changed 3 months ago by dgoulet

Reviewer: catalyst

comment:10 Changed 3 months ago by catalyst

Reviewer: catalystteor

comment:11 Changed 3 months ago by nickm

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

comment:12 Changed 3 months 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 months 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 months ago by ahf

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

comment:15 Changed 3 months 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 months 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 months ago by nickm (previous) (diff)

comment:17 Changed 3 months ago by dgoulet

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

comment:18 Changed 3 months 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 3 months ago by teor

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

comment:20 Changed 7 weeks ago by nickm

Type: defectenhancement

Mark a number of current 0.4.2.x "defects" as "enhancements."

comment:21 Changed 5 weeks ago by nickm

Keywords: 042-can added

comment:22 Changed 5 days ago by gaba

Can we close this ticket now?

comment:23 Changed 5 days ago by nickm

Resolution: fixed
Status: assignedclosed

I believe so; all child tickets are closed.

Note: See TracTickets for help on using tickets.