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

[Tor Bug Tracker & Wiki]

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

Comment (by efgyirfe784):

 My pleasure, glad I could help!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

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

 * status:  merge_ready => closed
 * resolution:   => fixed


Comment:

 Thanks for the additional review. Squashed both commits and made some
 trivial changes to the change log. Edits to the change log and squashing
 commits are things I'm happy to make as part of merging. Thanks again for
 the patch! Closing.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

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

 * status:  needs_review => merge_ready


Comment:

 Looks ready for merge. There are two commits but really the second commit
 should just be a fixup. karsten will fix this when merging.

 3530b53f3bb9c49bb089eb90869ff98fa7debb8c ok
 0d9d22caf28d9c772e65113fd6a2af182498622c fixup

 Thanks!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

#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:  irl  |Sponsor:
-+--
Changes (by irl):

 * status:  needs_information => needs_review


Comment:

 Ok thanks.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

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

Comment (by efgyirfe784):

 Sorry, you should be able to clone this:
 https://gitlab.com/743zpnpGUq27GgR/onionoo.git and look at branch
 '24419-isXY'.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

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

 * status:  needs_review => needs_information


Comment:

 I'm not exactly sure what I'm reviewing here.

 Can you give me a URL to clone and a branch name?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

#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:  irl  |Sponsor:
-+--

Comment (by efgyirfe784):

 I created child ticket #28114 to update metrics-lib.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

#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:  irl  |Sponsor:
-+--

Comment (by efgyirfe784):

 Done! (I think). I might've misunderstood "Append new change log entries
 to existing ones. " -> I moved the topmost changelog entry to just below
 the latest, but maybe this isn't what you were looking for?

 I think leaving the field name as "isRelay" makes sense.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

#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:  irl  |Sponsor:
-+--
Changes (by irl):

 * reviewer:   => irl


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

#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: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

#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:  karsten  |Sponsor:
-+--
Changes (by karsten):

 * status:  new => needs_review
 * reviewer:   => karsten


Comment:

 Reviewing now.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

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

Comment (by efgyirfe784):

 Please see https://gitlab.com/743zpnpGUq27GgR/onionoo/tree/24419-isXY for
 a proposed solution to this ticket.

 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. Note
 however that many docs classes had a "getMeasured()" method that is now
 "isMeasured()", which differs slightly from NetworkStatusEntry's
 "getUnmeasured()".

 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.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

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

[Tor Bug Tracker & Wiki]

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

 > The method name `getRecommendedVersion` is quite misleading - especially
 together with `getVersion` - as it returns a boolean not a version value,
 but as the method was introduced long ago this shouldn't halt a merge.
 >
 > Maybe, the naming could be improved at some point?

 So, what should we rename here? All `boolean`/`Boolean` fields in `docs`
 package classes to `isXY()`?

 Anything else?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs