Opened 6 months ago

Closed 6 months ago

#29884 closed defect (implemented)

Document how to edit practracker exception files

Reported by: teor Owned by: nickm
Priority: High Milestone: Tor: 0.4.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 040-must
Cc: asn, nickm Actual Points: .2
Parent ID: Points: 0.2
Reviewer: Sponsor:

Description

It's really not clear how I should edit practracker exception files.

We should update the documentation at the start of practracker.py, update the warning message logged by practracker.py, and maybe update HelpfulTools.md.

Marking this as 040-must, because it's blocking merges.

Child Tickets

TicketStatusOwnerSummaryComponent
#29912closedRegenerate practracker file with stable sort and control.c splitCore Tor/Tor

Change History (13)

comment:1 Changed 6 months ago by asn

Not sure if I will have time to do this in March. Perhaps someone else can take it.

comment:2 Changed 6 months ago by nickm

Owner: set to nickm
Priority: MediumHigh
Status: newaccepted

comment:3 Changed 6 months ago by nickm

Actual Points: .2
Status: acceptedneeds_review

I've made a branch called practracker_regen to do this, and to have a make target for regenerating the exceptions file. The PR is https://github.com/torproject/tor/pull/843 . We should probably merge this along with #29882.

comment:4 Changed 6 months ago by teor

These comments at the start of practracker.py are now outdated:

practracker.py should be run with its second argument pointing to the Tor
top-level source directory like this:
  $ python3 ./scripts/maint/practracker/practracker.py .
The exceptions file is meant to be initialized once with the current state of
the source code and then get saved in the repository for ever after:
  $ python3 ./scripts/maint/practracker/practracker.py . > ./scripts/maint/practracker/exceptions.txt

Do we also need to update HelpfulTools.md?

comment:5 Changed 6 months ago by teor

Status: needs_reviewneeds_revision

comment:6 Changed 6 months ago by teor

Should we regenerate the practracker exceptions file in this branch?
If we commit the new stable sort order now, future regenerations will have cleaner diffs.

comment:7 Changed 6 months ago by nickm

Let's not regenerate until after #29894 is merged -- it will save us some conflicts.

comment:8 Changed 6 months ago by nickm

Status: needs_revisionneeds_review

I've tweaked the comment at the head of the file.

comment:9 in reply to:  7 Changed 6 months ago by nickm

Replying to nickm:

Let's not regenerate until after #29894 is merged -- it will save us some conflicts.

#29894 is now merged (thanks, asn!). Let's regenerate once this branch is merged too.

comment:10 Changed 6 months ago by teor

Status: needs_reviewmerge_ready

Ok, I am happy with this ticket.

I will open child tickets for the remaining issues.

comment:11 Changed 6 months ago by nickm

I've merged the branch to master.

comment:12 Changed 6 months ago by teor

Un-parented open child, so we can close.

comment:13 Changed 6 months ago by nickm

Resolution: implemented
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.