[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