Re: [tor-bugs] #19021 [Metrics/CollecTor]: improve configuration process

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

#19021: improve configuration process
-------------------------------+-----------------------------
 Reporter:  iwakeh             |          Owner:  iwakeh
     Type:  enhancement        |         Status:  merge_ready
 Priority:  High               |      Milestone:
Component:  Metrics/CollecTor  |        Version:
 Severity:  Normal             |     Resolution:
 Keywords:  ctip operation     |  Actual Points:
Parent ID:                     |         Points:
 Reviewer:                     |        Sponsor:
-------------------------------+-----------------------------
Changes (by karsten):

 * status:  needs_review => merge_ready


Comment:

 Alright, I made it through that patch and ran some local tests.
 Surprisingly (to myself) I don't have a single change request!  I briefly
 thought whether CollecTor should really write the `collector.properties`
 file if it doesn't exist, because that's a bit unexpected, but I couldn't
 think of a situation where that would be harmful.

 A minor remark though for future patches of similar size: Can you split up
 big commits that change more than one thing into several smaller commits?
 Whenever I find myself writing a huge patch, I go through the diff and try
 to find related changes and give them numbers.  Then I do `git reset
 HEAD^` and run `git add -p` and `git commit` until all changes are
 committed.  There are more advanced ways to split up commits using `git
 rebase -i`, but in this case that would not have been necessary.  The
 beauty of smaller commits is that they're easier to review.  Though I
 don't know whether splitting it up would have resulted in several commits
 of similar size or one almost-as-big commit and a couple of tiny ones.
 See the recent Javadoc change to metrics-lib where I separated even tiny
 code changes from the Javadoc commit.

 So, should I still wait with merging this to master?  Do you expect this
 commit to change once the scheduler implementation gets in?  If not, why
 not merge now?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/19021#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