monero-project / kovri

The Kovri I2P Router Project
Other
571 stars 114 forks source link

[HOLD] Tunnel: tunnel config unit-tests & functional refactors for testing and correctness #987

Closed coneiric closed 6 years ago

coneiric commented 6 years ago

By submitting this pull-request, I confirm the following:

coneiric commented 6 years ago

I gave you complete free reign to lead the boost.beast migration

Happy to focus on this. My understanding was that the migration was for after release.

This PR does not help and does not fix anything.

It provides 86% coverage to a completely untested module, fixes some logic for not setting both Gateway and Endpoint tunnel flags simultaneously, and addresses some small TODOs in the code. It is not intended to fix the overarching refactor work that still needs to be done.

You're also repeatedly violating https://google.github.io/styleguide/cppguide.html#Aliases

According to that portion of the style guide, aliases can be used to " help the implementation retain some degree of freedom to change the alias". One of the examples even has an alias used purely for user convenience. The aliases I use are for convenience, readability, and to help with the constants refactor noted in core/router/i2np.h. I don't see how this is a failure to understand how to use aliases.

It's obvious now that most of the work I have been submitting lately is only adding stress. That's not my intention, and I have just been trying to follow through on my FFS commitments. If the best way for me to do that is to stop writing tests, and move to focusing on the Boost.Beast transition, then I'll do that.

anonimal commented 6 years ago

NOTICE: THIS PULL REQUEST HAS BEEN MOVED TO GitLab. Please reopen there if you wish to merge. See https://github.com/monero-project/kovri/issues/1013 for details.