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

["Tor Bug Tracker & Wiki" <blackhole@xxxxxxxxxxxxxx>]

#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 karsten):

 Replying to [comment:8 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.

 Alright, those are good arguments for changing to 80.  Changed in the
 docs.

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

 Yep, I ran that, too, and it didn't complain.  Okay, removed the newlines
 anyway.

 > The three sentence javadoc will become one paragraph, but when editing
 it later it's easier to see, which sentence was changed.

 Right, I assumed that the first paragraph should be a single sentence,
 because that's what goes into the summary.  I didn't change this yet, but
 what do you think about the suggestion above?

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

 Changed.

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

 Sounds good.  Amended and pushed to [https://gitweb.torproject.org/karsten
 /metrics-db.git/log/?h=task-18865 my task-18865 branch].

 Can you add this format guideline to the docs?

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

 Great! :)  As soon as I know what to do with the Javadoc, I'll push and
 close.  Thanks!

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18865#comment:9>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs