Re: [tor-bugs] #18955 [Metrics/CollecTor]: javadoc coverage checkstyle warnings

[Tor Bug Tracker & Wiki]

#18955: javadoc coverage checkstyle warnings
---+-
 Reporter:  iwakeh |  Owner:  iwakeh
 Type:  task   | Status:  closed
 Priority:  Medium |  Milestone:  CollecTor 1.0.0
Component:  Metrics/CollecTor  |Version:
 Severity:  Normal | Resolution:  fixed
 Keywords:  ctip   |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+-
Changes (by iwakeh):

 * milestone:   => CollecTor 1.0.0


--
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] #18955 [Metrics/CollecTor]: javadoc coverage checkstyle warnings

[Tor Bug Tracker & Wiki]

#18955: javadoc coverage checkstyle warnings
---+
 Reporter:  iwakeh |  Owner:  iwakeh
 Type:  task   | Status:  closed
 Priority:  Medium |  Milestone:
Component:  Metrics/CollecTor  |Version:
 Severity:  Normal | Resolution:  fixed
 Keywords:  ctip   |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+
Changes (by iwakeh):

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


Comment:

 Right!

 I guess I have to work on my question formulation ;-)

 Closing.  Thanks for the discussion.

--
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] #18955 [Metrics/CollecTor]: javadoc coverage checkstyle warnings

[Tor Bug Tracker & Wiki]

#18955: javadoc coverage checkstyle warnings
---+--
 Reporter:  iwakeh |  Owner:  iwakeh
 Type:  task   | Status:  needs_review
 Priority:  Medium |  Milestone:
Component:  Metrics/CollecTor  |Version:
 Severity:  Normal | Resolution:
 Keywords:  ctip   |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+--

Comment (by karsten):

 The question is: do we still need to do anything, now that we're down to
 zero warnings?

--
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] #18955 [Metrics/CollecTor]: javadoc coverage checkstyle warnings

[Tor Bug Tracker & Wiki]

#18955: javadoc coverage checkstyle warnings
---+--
 Reporter:  iwakeh |  Owner:  iwakeh
 Type:  task   | Status:  needs_review
 Priority:  Medium |  Milestone:
Component:  Metrics/CollecTor  |Version:
 Severity:  Normal | Resolution:
 Keywords:  ctip   |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+--

Comment (by iwakeh):

 Replying to [comment:7 karsten]:
 > Suppressing warnings can be good (for example, for attribute names
 containing `_` which we cannot change), but I'd rather want to write the
 missing Javadocs than put in something to suppress the warning.  Depends
 on the number of warnings, of course. :)

 Agreed.
 In order to handle huge amounts of warnings we arrived at the idea to have
 two checkstyle tasks (comments 3 and 4).  Thinking more about it the
 suppress warnings seams to be the better solution.
 The addition of a new annotation will be clearly noticeable when doing a
 review and could be rejected. It is always visible in the code as a
 permanent reminder, but will not be cluttering the checkstyle report
 anymore.  So, the question is whether there still should be two checkstyle
 tasks?

--
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] #18955 [Metrics/CollecTor]: javadoc coverage checkstyle warnings

[Tor Bug Tracker & Wiki]

#18955: javadoc coverage checkstyle warnings
---+--
 Reporter:  iwakeh |  Owner:  iwakeh
 Type:  task   | Status:  needs_review
 Priority:  Medium |  Milestone:
Component:  Metrics/CollecTor  |Version:
 Severity:  Normal | Resolution:
 Keywords:  ctip   |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+--

Comment (by karsten):

 Suppressing warnings can be good (for example, for attribute names
 containing `_` which we cannot change), but I'd rather want to write the
 missing Javadocs than put in something to suppress the warning.  Depends
 on the number of warnings, of course. :)

--
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] #18955 [Metrics/CollecTor]: javadoc coverage checkstyle warnings

[Tor Bug Tracker & Wiki]

#18955: javadoc coverage checkstyle warnings
---+--
 Reporter:  iwakeh |  Owner:  iwakeh
 Type:  task   | Status:  needs_review
 Priority:  Medium |  Milestone:
Component:  Metrics/CollecTor  |Version:
 Severity:  Normal | Resolution:
 Keywords:  ctip   |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+--
Changes (by iwakeh):

 * status:  accepted => needs_review


Comment:

 Actually, I think it's better to use suppression annotations.

 {{{
 #!java

   @SuppressWarnings("checkstyle:javadocmethod")
   public static void main(Configuration config) throws
 ConfigurationException {
 }}}

 First added to CollecTor
 [https://gitweb.torproject.org/user/iwakeh/collector.git/commit/?h=task-19615
 -guide-compliance=26c3449b1835b6807ed09ebcda6809f6e9b30f4f here] as
 part of guide compliance #19615.

 This way it is visible in the code where exemptions were made.

 Is this ok?

--
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] #18955 [Metrics/CollecTor]: javadoc coverage checkstyle warnings (was: javadoc coverage)

[Tor Bug Tracker & Wiki]

#18955: javadoc coverage checkstyle warnings
---+--
 Reporter:  iwakeh |  Owner:  iwakeh
 Type:  task   | Status:  accepted
 Priority:  Medium |  Milestone:
Component:  Metrics/CollecTor  |Version:
 Severity:  Normal | Resolution:
 Keywords:  ctip   |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+--
Changes (by iwakeh):

 * status:  new => accepted


Comment:

 clarified title.
 added to my list; todo cf. comment:4

--
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] #18955 [Metrics/CollecTor]: javadoc coverage

[Tor Bug Tracker & Wiki]

#18955: javadoc coverage
---+
 Reporter:  iwakeh |  Owner:  iwakeh
 Type:  task   | Status:  new
 Priority:  Medium |  Milestone:
Component:  Metrics/CollecTor  |Version:
 Severity:  Normal | Resolution:
 Keywords:  ctip   |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+

Comment (by iwakeh):

 Fine, I'll split the checks and add the new task.

--
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] #18955 [Metrics/CollecTor]: javadoc coverage

[Tor Bug Tracker & Wiki]

#18955: javadoc coverage
---+
 Reporter:  iwakeh |  Owner:  iwakeh
 Type:  task   | Status:  new
 Priority:  Medium |  Milestone:
Component:  Metrics/CollecTor  |Version:
 Severity:  Normal | Resolution:
 Keywords:  ctip   |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+

Comment (by karsten):

 Okay, want to split the checks file and add another Ant task?  I guess we
 can always reverse that change later and go back to a single checks file
 when our javadocs are in better shape and we're optimistic that they'll
 stay that way.  (The advantage of making as few changes as possible to the
 checks file is that it'll be easier to update to a newer version.)

--
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] #18955 [Metrics/CollecTor]: javadoc coverage

[Tor Bug Tracker & Wiki]

#18955: javadoc coverage
---+
 Reporter:  iwakeh |  Owner:  iwakeh
 Type:  task   | Status:  new
 Priority:  Medium |  Milestone:
Component:  Metrics/CollecTor  |Version:
 Severity:  Normal | Resolution:
 Keywords:  ctip   |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+

Comment (by iwakeh):

 In general, that sounds fine to me.

 It might be useful to separate the javadoc checks from all the other
 checkstyle checks, i.e. have another ant task `check-javadoc`.
 The current `metrics_checks.xml` could be split into
 `metrics_checks_javadoc.xml` (with just javdoc checks) and
 `metrics_checks.xml` with the other checks.
 This way there is no cluttering with the javadoc warnings, but the check
 is available. And, this kind of separation will be useful in future, too.

 >  ... rephrasing Javadoc that doesn't start with a summary sentence
 (happy to conform to that rule if it's in the guide) ...
 The summary sentence should start with a summary sentence/fragment, cf.
 [https://google.github.io/styleguide/javaguide.html#s7.2-summary-fragment
 Style Guide].

 Good news:
 We do not need to change the style guide for not writing javadoc for
 obvious (e.g. getter and setter) methods, cf.
 [https://google.github.io/styleguide/javaguide.html#s7.3.1-javadoc-
 exception-self-explanatory here].

--
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] #18955 [Metrics/CollecTor]: javadoc coverage

[Tor Bug Tracker & Wiki]

#18955: javadoc coverage
---+
 Reporter:  iwakeh |  Owner:  iwakeh
 Type:  task   | Status:  new
 Priority:  Medium |  Milestone:
Component:  Metrics/CollecTor  |Version:
 Severity:  Normal | Resolution:
 Keywords:  ctip   |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+

Comment (by karsten):

 Thanks for starting this discussion.  Two suggestions:
  1. We make the easy changes now.  That includes turning comments into
 Javadoc, rephrasing Javadoc that doesn't start with a summary sentence
 (happy to conform to that rule if it's in the guide), and maybe others
 that require less than a minute per comment.
  1. We defer the non-easy changes and general decisions until we have
 finished the metrics-lib Javadoc ticket (#16873) and made bigger
 refactoring measures like removing dead code.  This could take a few
 weeks, and it doesn't make much sense to clutter the checkstyle output
 with warnings we cannot fix right now.  We should mention in the
 checkstyle document that this is a temporary measure.

 How does that sound?

--
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