TL;DR for potential reviewers: Unit tests not reviewed yet. Code is here: [1] Please add an "ACK upon this file" to the end of each file.
Hello bertm,
the background jobs [0] review [1] is now finished for everything except the
unit tests - it might make sense for me to wait with reviewing the unit tests
until the remaining changes to the core are done, so you get this mail
already.
I can proceed to review the tests nevertheless if you want to work off the
suggested changes in batch including unit tests.
If someone else wants to step in to review the tests, so I have more time
available for finishing the remaining WOT blockers, feel free to do so.
Please add an "ACK upon this file" to the end of each file which you have
reviewed.
In case anyone is interested in even further work: I have discovered some
weirdness in the backend fred code, which I commented upon at [4] and [5].
For the review results please notice two things:
- I am sort of ashamed of having done an insane amount of comments. As always,
please don't feel like I'm trying to nitpick. I merely want to provide you as
much service as I can by writing down everything which comes to my mind during
review.
- I have already added "Not a blocker" to many comments which you don't have
to change if you don't want to. Looking back at how much time it took me to
review this already (reviewing threaded code is more work than I had
imagined), I would like to amend the list of things which you don't have to
change, to reduce further time needed for reviewing the changes. The amendment
is as follows: Please postpone major refactoring to a secondary pull request.
"Major" refactoring = such which would be complex enough to cause me to have
to review the threading-correctness again. Good examples for this are comments
[2] and [3], but it is up to you to decide to postpone further stuff. Add a
comment to my relevant comment in case you want to do that.
Besides that, please notice my huge thanks and respect for the pull request.
Subjectively I would estimate that you wrote the code faster than I was able
to review it, which shows your expertise :)
In total you saved me a lot of WOT work, thanks!
Greetings,
xor
[0] https://github.com/freenet/plugin-WebOfTrust/pull/26
[1] https://github.com/freenet/plugin-WebOfTrust/pull/26/files
[2] https://github.com/freenet/plugin-WebOfTrust/pull/26/files#r22519360
[3] https://github.com/freenet/plugin-WebOfTrust/pull/26/files#r22583312
[4]
https://github.com/freenet/fred/commit/cd3af19f0d3390872edf062ac95f6e8fa13986f2
[5]
https://github.com/freenet/fred/commit/2f6107b093d9b0229722fcab18ad416c8633227b
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Devl mailing list Devl@freenetproject.org https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
