[tor-bugs] #18865 [Metrics/CollecTor]: actively monitor resources like available storage space

[Tor Bug Tracker & Wiki]

#18865: actively monitor resources like available storage space
-------------------------------+---------------------------------
 Reporter:  iwakeh             |          Owner:  iwakeh
     Type:  enhancement        |         Status:  needs_revision
 Priority:  Medium             |      Milestone:  CollecTor 1.0.0
Component:  Metrics/CollecTor  |        Version:
 Severity:  Normal             |     Resolution:
 Keywords:  ctip               |  Actual Points:
Parent ID:                     |         Points:
 Reviewer:                     |        Sponsor:
-------------------------------+---------------------------------

Comment (by iwakeh):

 Thanks for reviewing so quickly!

 >  - Some lines exceed the implicit 74 character limit that I have been
 using in the past and that we might not have included in the styleguides
 yet.  Let me explain: 74 characters is the maximum number of characters
 that look reasonable in vim with line numbers turned on in files up to 999
 lines.  Somewhat arbitrary, I know.  Are there arguments in favor of other
 line lengths?  If so, let's discuss those.  Otherwise, let's just keep 74.

 Well, we actually have a
 [https://trac.torproject.org/projects/tor/wiki/org/teams/MetricsTeam/MetricsJavaStyleGuide#s4.4
 -column-limit 80-100] column limit, but I prefer to keep it between 70 and
 80.  That's why this shows up repeatedly.

 Please, make the changes to the guide doc, but I prefer 80.

 (I vaguely remember that we agreed on 76-80 in one meeting, but I can't
 find the minutes.)

 74 is too short and the problem will become worse with java8 and using
 lambda expressions and more fluent style.  The coding style changed and we
 all use screens way bigger than in the last century.  A line of length 80
 can easily be read with one glance still.


 >
 >  - Can we remove those newlines after closing brackets?
 >
 >  - I wonder how the Javadoc with three sentences without newline between
 first and second and without <p></p> around second and third sentence
 conforms with checkstyle.  Can we rephrase those three sentences to a
 single sentence or separate them into one sentence and a paragraph?

 I usually run `ant checks` before committing and it doesn't complain.
 The three sentence javadoc will become one paragraph, but when editing it
 later it's easier to see, which sentence was changed.

 >
 >  - Both `1024.` and `Math.floor` seem unnecessary for `long` values.
 Can we change them to `1024` and omit the flooring?

 Oops, please change.

 >
 >  - I think the preferred Git message format is: "summary of no more than
 50 chars, newline, one or more parapraphs with no more than 70 chars per
 line".  (Note: I sometimes exceed the 50, and I'm open to using something
 else than 70, for example, 72 which I just read on a random blog post.)
 Do you mind if we change the message to something like the following?
 >
 > {{{
 > Check available disk space in relaydescs module.
 >
 > Check if there's enough space before and after running the relaydescs
 > module, and warn if less than 200 MiB are left.
 >
 > Implements #18865.
 > }}}

 That's fine!
 And the git-msg format should have a place in the docs somewhere.

 >
 > So, mostly whitespace complaints. :)  But I figured it's better to bring
 them up at some point to make future reviews faster.  Again, happy to
 change things if you agree.

 Please do. Thanks a lot!

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