Merged
Conversation
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
ae2a2df to
1ff14d2
Compare
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
622da58 to
72d0678
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #690 +/- ##
===========================================
+ Coverage 74.06% 77.71% +3.64%
===========================================
Files 145 158 +13
Lines 8958 8711 -247
===========================================
+ Hits 6635 6770 +135
+ Misses 2323 1941 -382 ☔ View full report in Codecov by Sentry. |
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
840af5b to
7bec6a3
Compare
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
f922e82 to
65bc3c1
Compare
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
65bc3c1 to
0d35075
Compare
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
087b785 to
9f107e7
Compare
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
7a10305 to
d69731b
Compare
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
d69731b to
3dd693d
Compare
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
69c0137 to
6ed6195
Compare
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
78c66f6 to
f7f4f37
Compare
extracted async and sync implementations
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
d81b693 to
43283b7
Compare
da2ce7
force-pushed
the
20240209_ordered_repo
branch
from
43283b7 to
3414e2a
Compare
josecelano
approved these changes
Mar 25, 2024
Member
josecelano
left a comment
josecelano
left a comment
There was a problem hiding this comment.
Hi @da2ce7
This is a lot of work and it was needed. Just a couple of generic comments:
- You could have created at least 3 different PRs: two of them to extract the new packages (clock and torrent-repository-xx) and another one for the rest (primitives). Just to avoid wasting time merging potential conflicts. For example, merging this PR before this one.
- There are things I would move to independent repos. Basically, the things that can be used in other repos not only the tracker.
- clock. See: https://github.com/torrust/torrust-index/blob/develop/src/utils/clock.rs
- infohash. See torrust/torrust-index#251
But we can do that later. I've created a new issue from one of my old comments about this topic:
Member
There was a problem hiding this comment.
Hi @da2ce7 why didn't you move the tests also to the torrust_tracker_primitives crate?
Member
|
ACK 3414e2a |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request is a major refactor of the torrent repository for the tracker. The code for the repository has been extracted and merged into a library package, along side it's benchmarking and testing code.
The supporting clock and time extent, and some other primitives have also been extracted, to avoid circular dependencies.