Re: [tor-bugs] #23100 [Core Tor/Tor]: Circuit Build Timeout needs to count hidden service circuits

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

#23100: Circuit Build Timeout needs to count hidden service circuits
-------------------------------------------------+-------------------------
 Reporter:  mikeperry                            |          Owner:
                                                 |  mikeperry
     Type:  enhancement                          |         Status:
                                                 |  needs_revision
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.3.x-final
Component:  Core Tor/Tor                         |        Version:  Tor:
                                                 |  0.2.7
 Severity:  Normal                               |     Resolution:
 Keywords:  tor-hs, path-bias, guard-discovery-  |  Actual Points:
  prop247-controller, needs-proposal, mike-can,  |
  prop247, tor-guard, review-group-25            |
Parent ID:  #9001                                |         Points:
 Reviewer:  asn                                  |        Sponsor:
-------------------------------------------------+-------------------------

Comment (by mikeperry):

 asn - I am trying to honor your many requests for things that were either
 already done, redundant to other code, and/or difficult to deal with wrt
 git history.

 Since these two tickets touch the very same block of code, I see them as a
 unit. Maybe I should have simply filed only one ticket... Separating them
 is very time consuming, especially wrt tests (which did not exist for any
 of this code before, so I ended up having to write tests that covered a
 lot of timeout functionality - which was also changing in *both* of these
 tickets). If you want me to separate the tests, and refactor separately,
 and do the copy-code-first thing (even though in my eyes, that is what
 #213100 mostly was, aside from one extra conditional and a relocation),
 that will take me another *two days* of git juggling, test re-writing, and
 verification, at least.

 I spent all day yesterday trying to merge the fixups for #23100 down
 without conflicts for #23114. Other than the refactoring you asked for, in
 fact all of those fixups are cleanly merged into #23100. I even provided
 you two versions of the branch (pre-fixup and post-fixup) to verify this.

 Wrt refactoring: because you requested that the initial commit just be a
 move of code with minimal changes, I merged the refactoring that you asked
 for into the following ticket, #23114. I think the refactoring was a good
 idea. But to avoid making you review a completely different #23100, I did
 it in #23114 instead... It is frustrating to me that you are now upset by
 this effort.

 I think we are both underestimating what we're asking of the other, and
 communicating poorly on top of it. :/

 I can make a separate commit for this, but I think it is way too much for
 you to also ask for me to separate the tests for this and #23114 just for
 git history. And I'm not sure if you are even asking for this. That is
 still not clear.

 If you want to merge this without tests, that's fine. I will make a branch
 for the #23100 commit in a second, and I will also make a reference branch
 with only the fixups it contains. But if you want tests with this code,
 again, I would rather simply wait for someone to review #23114. As I said,
 the tests cover a lot of timeout functionality that is being changed by
 these tickets. Writing tests for each ticket means changing the tests
 after each ticket, which is a lot of busy work for no gain in the end.

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