[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #21651 [Core Tor/Tor]: prop140/compression: Refactor directory cache spooling code



#21651: prop140/compression: Refactor directory cache spooling code
-------------------------------------------------+-------------------------
 Reporter:  nickm                                |          Owner:  nickm
     Type:  defect                               |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.1.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  TorCoreTeam201703, prop140, review-  |  Actual Points:  1
  group-17                                       |
Parent ID:                                       |         Points:  3
 Reviewer:  ahf                                  |        Sponsor:
                                                 |  Sponsor4
-------------------------------------------------+-------------------------
Changes (by asn):

 * status:  needs_review => needs_revision


Comment:

 Hello, I did a review for this ticket on gitlab. Please check it out! :)

 Also, I must say that the commit structure was not very helpful to review,
 especially since I'm not familiar with that part of the codebase
 (dirserv/connection logic):

 `8587f66` was pretty small and nice, but `6316f23` did everything and the
 kitchen sink. It moved code, it removed code and it added new code without
 splitting it into separate commits. So I had to continuously check between
 the code and the diff to see whether what I'm reviewing is new or old and
 how it used to be. I think if the branch was split into 3-4 commits it
 would be easier to review (e.g. one commit introduces the spooling
 structures and basic API, another commit moves old code to new functions,
 another commit edits such code, another commit introduces new code).

 Hope the review is useful!

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