#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@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs