Opened 16 months ago

Closed 6 hours ago

#29223 closed task (wontfix)

List canonical abbreviations to use in Tor functions and identifiers

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.4.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: doc, s31-docs 042-can
Cc: Actual Points: 0.9
Parent ID: Points: 1
Reviewer: asn Sponsor:

Description

We often don't abbreviate things in our function names and global identifiers. This leads to beautiful things like "router_get_mutable_consensus_status_by_descriptor_digest" and "signed_descs_update_status_from_consensus_networkstatus" and "networkstatus_consensus_can_use_multiple_directories".

What's more, when we do abbreviate, we often do so inconsistently: cfg versus config, con vs conn, orconn vs or_conn, and so on.

We should specify which abbreviations we want to standardize, and converge on those.

It is most important to do this for identifiers that are visible across modules, followed by identifiers that are global within a module. I don't think we need to do this for local variables.

Child Tickets

Attachments (2)

abbrevs-today.txt (8.2 KB) - added by nickm 14 months ago.
abbrevs-proposed.txt (9.4 KB) - added by nickm 14 months ago.

Download all attachments as: .zip

Change History (29)

comment:1 Changed 14 months ago by gaba

Points: 0.51

comment:2 Changed 14 months ago by nickm

Milestone: Tor: 0.4.1.x-final

comment:3 Changed 14 months ago by gaba

Sponsor: Sponsor31-must

Changed 14 months ago by nickm

Attachment: abbrevs-today.txt added

comment:4 Changed 14 months ago by nickm

I've attached a file documenting all the abbreviations that are currently in use in function names. Some of them collide; some are ambiguous; some long terms in frequent use are missing abbreviations.

Next steps here are to identify preferred versions (or not) for all ambiguous abbreviations; decide what to rectify and what to grandfather in; and make a (proposed) canonical list.

Changed 14 months ago by nickm

Attachment: abbrevs-proposed.txt added

comment:5 Changed 14 months ago by nickm

Actual Points: .7
Status: newneeds_review

I've attached a second file, proposing to standardize on certain abbreviations, add new ones, and decline to abbreviate other things. I'm hoping that we come up with a good compromise between readability and terseness, though I recognize that no compromise will be optimal for everybody.

comment:6 Changed 14 months ago by nickm

Owner: set to nickm
Status: needs_reviewaccepted

comment:7 Changed 14 months ago by nickm

Status: acceptedneeds_review

comment:8 Changed 14 months ago by asn

Reviewer: teor

comment:9 Changed 14 months ago by teor

I want to make comments on this file, and I'd usually do that through a pull request.
Where is it going to live?
I think I'll start by dumping it in doc/HACKING, and we can move it to its final location later,

I also have a broader question:

Are identifiers in the tor specifications in scope?
Juga and I spent a lot of time trying to name fields for the bandwidth file.
A set of standard abbreviations would have been very useful.

comment:10 in reply to:  9 Changed 14 months ago by nickm

Replying to teor:

I want to make comments on this file, and I'd usually do that through a pull request.
Where is it going to live?

I don't know yet; I was undecided between doc/HACKING, the wiki, or something else.

I think I'll start by dumping it in doc/HACKING, and we can move it to its final location later,

That's fine.

I also have a broader question:

Are identifiers in the tor specifications in scope?
Juga and I spent a lot of time trying to name fields for the bandwidth file.
A set of standard abbreviations would have been very useful.

This list does not try to cover the specification, but having a consistent set of abbreviations and symbols to use there would be good.

comment:11 Changed 14 months ago by teor

I have reviewed about half of this list, I'll submit my review now. I'll finish the rest tomorrow.

comment:12 Changed 14 months ago by teor

Actual Points: .70.9
Reviewer: teor

I have reviewed the other half of the list.

I made some general comments on the pull request, and some specific comments against particular abbreviations.

I am not sure how we plan to converge on these abbreviations. Knowing the use cases will help future reviewers give more helpful advice.

I am removing myself as a reviewer, because I will be on leave from the middle of this week.

comment:14 Changed 14 months ago by asn

Reviewer: catalyst

comment:15 Changed 13 months ago by nickm

Keywords: doc added

Mark a few documentation tickets as "doc"

comment:16 Changed 13 months ago by nickm

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

comment:17 Changed 10 months ago by teor

Reviewer: catalystteor
Status: needs_reviewneeds_revision

I spoke with catalyst about these new abbreviations.

  • Do we have an (automated?) migration plan to the new abbreviations?
  • Can we split the patch into topic areas (or modules)?
  • How do we enforce the new abbreviations after migration?

Answering these questions should help us move forward, and it will help me review the final list,

comment:18 Changed 10 months ago by nickm

I think my answer for all three questions is "I don't know". I can try to do so wrt the topic areas suggectino, but I don't have much of an idea how to migrate or enforce them. Any suggestions? Should we bring it up with the team?

comment:19 Changed 9 months ago by nickm

Keywords: s31-docs added

comment:20 Changed 9 months ago by nickm

Sponsor: Sponsor31-mustSponsor31-can

comment:21 Changed 9 months ago by nickm

Keywords: 042-can added

comment:22 Changed 9 months ago by teor

I think the scope of this ticket might be too big.

Let's try to tackle it in a few different parts:

  • Use the same abbreviations for
    • module names (we might already do this)
    • source file names (we might already do this, but I bet there are a few exceptions)
    • functions used outside their module
    • functions in header files
    • other functions

I don't know if we can enforce or automate these changes.

I don't know how important this ticket is. Maybe we could decide to do other things for this sponsor.

comment:23 Changed 4 months ago by gaba

Sponsor: Sponsor31-can

No more sponsor 31. All this tickets remained open after sponsor 31 ended.

comment:24 Changed 2 weeks ago by nickm

Milestone: Tor: 0.4.2.x-finalTor: 0.4.4.x-final
Reviewer: teor
Status: needs_revisionneeds_review

I'll need some feedback from the current reviewers: I'm wondering whether we think this is still a useful thing to do, and what we should do with it, if anything.

comment:25 Changed 10 days ago by asn

Reviewer: asn

comment:26 Changed 8 days ago by asn

Status: needs_reviewneeds_information

I'm inclined to NACK this branch. I think such a big list of abbreviations will be hard to maintain and use both for us and for volunteers. Also I feel like the benefit does not outweight the overhead of enforcing the abbreviations and checking whether an abbreviation exists everytime you want to write code.

Perhaps as a way to do short-term progress here would be to identify a bunch of important abbreviations that we get wrong often and fix those.

And then if in the future this abbreviation inconsistency issue becomes a problem, we can revisit the list from comment:13 and see how to make it useful.

Or we can ignore this ticket for now (which honestly would be my approach).

comment:27 Changed 6 hours ago by nickm

Resolution: wontfix
Status: needs_informationclosed

Okay, I think you're right here. I don't know how to use this, but I'll remember how to find it in case we someday decide to resurrect the effort.

Note: See TracTickets for help on using tickets.