[tor-bugs] #24419 [Metrics/Onionoo]: Improve getter names for boolean fields

Tor Bug Tracker & Wiki blackhole at torproject.org
Mon Oct 15 14:16:23 UTC 2018


#24419: Improve getter names for boolean fields
-----------------------------+------------------------------
 Reporter:  karsten          |          Owner:  metrics-team
     Type:  enhancement      |         Status:  needs_review
 Priority:  Medium           |      Milestone:
Component:  Metrics/Onionoo  |        Version:
 Severity:  Normal           |     Resolution:
 Keywords:                   |  Actual Points:
Parent ID:                   |         Points:
 Reviewer:                   |        Sponsor:
-----------------------------+------------------------------
Changes (by karsten):

 * reviewer:  karsten =>


Comment:

 Replying to [comment:1 efgyirfe784]:
 > Please see https://gitlab.com/743zpnpGUq27GgR/onionoo/tree/24419-isXY
 for a proposed solution to this ticket.

 Looks good and is almost ready to be merged. The only changes I'd make
 are:
  - Append new change log entries to existing ones.
  - Start the new change log entry with a capital letter and end it with a
 period.
  - Squash the second commit into the first, because it only removes a file
 that was accidentally added in the first commit.

 > Please note:
 >
 > 1. I did not change
 src/test/org/torproject/metrics/onionoo/updater/DummyStatusEntry's
 "getUnmeasured()" to "isUnmeasured()" because the interface
 org.torproject.descriptor.NetworkStatusEntry requires it, and I figured
 org.torproject.descriptor is outside the scope of this ticket.

 Agreed. We'd first have to change metrics-lib interfaces, put out a new
 major metrics-lib release, and then update the tests in Onionoo. Worth
 doing, though we would ideally combine this with other changes that
 require a major metrics-lib version bump. '''Let's create a ticket before
 closing this one.'''

 > Note however that many docs classes had a "getMeasured()" method that is
 now "isMeasured()", which differs slightly from NetworkStatusEntry's
 "getUnmeasured()".

 That's two different thing. One says ''whether'' a relay was measured, the
 other says what the ''measurement result'' was.

 > 2. Several classes in org.torproject.metrics.onionoo.docs (like
 SummaryDocument) have a field called "isRelay" which has a getter method
 named "isRelay()". I think this makes sense because of what the field
 represents, but some folks prefer not to have field names repeated as
 method names. I didn't change this, just an FYI.

 We could maybe rename the field to `relay`. Or leave it as it is.

 I'll again leave this here for irl to review, too, before merging it to
 master with the minor changes listed above. Thanks!

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/24419#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online


More information about the tor-bugs mailing list