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

Re: [tor-bugs] #8510 [Tor]: Add useful Hidden Service related events to the Tor control port

#8510: Add useful Hidden Service related events to the Tor control port
-------------------------+----------------------------------------------
     Reporter:  hellais  |      Owner:
         Type:  task     |     Status:  needs_revision
     Priority:  normal   |  Milestone:  Tor: unspecified
    Component:  Tor      |    Version:
   Resolution:           |   Keywords:  tor-hs controller needs-proposal
Actual Points:           |  Parent ID:  #8993
       Points:           |
-------------------------+----------------------------------------------
Changes (by asn):

 * status:  needs_review => needs_revision


Comment:

 Ah, nice! We are getting closer!

 Some comments on the unit test:
 * Can you add some '''brief''' docs to specify what the test is supposed
 to do?
 * The `received_msg` global variable is a bit confusing -- especially if
 we expect to add more unit tests to `test_hs.c`. Can you at least document
 that the variable is a helper for a specific test? Same goes for the
 `STR_HS_ADDR` constants, which I would personally only define within the
 body of `test_hs_desc_event()`.
 * Check out `test_streq()` instead of `test_assert(!strncmp(...))`.
 * Also, you merged master on top of your branch. Ideally, your commits
 should be on top and you should rebase your branch to use master as its
 base.

 If you don't have time to do the above tasks, just tell us and we can do
 them before merging.

 Thanks!

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