[metrics-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 metrics-bugs
mailing list