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

 * status:  needs_review => needs_revision


Comment:

 Change looks good, just minor nitpicks, and I'm happy to make all changes
 if you're okay with that:

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

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

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

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

 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.

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