Try & simplify Registry vs Source confusion #5476
Conversation
For now just move `supports_checksums` and `requires_precise` methods down from `Registry` to `Source`, as they're not as entangled as `query`.
.. and move query definitions to the struct impls.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
I just had a thought: is it possible to delegate from one trait to the other, rather than have both traits delegate to the struct? |
src/cargo/core/registry.rs
Outdated
|
|
||
| /// Returns whether or not this registry will return summaries with | ||
| /// the `precise` field in the source id listed. | ||
| fn requires_precise(&self) -> bool; | ||
| } | ||
|
|
||
| impl<'a, T: ?Sized + Registry + 'a> Registry for Box<T> { |
There was a problem hiding this comment.
We can remove this impl altogether.
src/cargo/sources/directory.rs
Outdated
| @@ -46,12 +55,13 @@ impl<'cfg> Debug for DirectorySource<'cfg> { | |||
|
|
|||
| impl<'cfg> Registry for DirectorySource<'cfg> { | |||
There was a problem hiding this comment.
I think we now can remove all impl Registry for XSource blocks.
|
Looks like a good start, but now I believe we can remove a whole bunch of useless impls! |
Optimised for minimised PR diff, and thus reviewability
|
You're right, @matklad! I hadn't realised. That's much better. My first net 🔻 PR! 🎉 |
src/cargo/core/registry.rs
Outdated
| @@ -703,13 +673,5 @@ pub mod test { | |||
| Ok(()) | |||
There was a problem hiding this comment.
@matklad Shall I trash this impl Registry for RegistryBuilder in cfg(test)?
I thought for a while it might be there to, perhaps, assert at compile-time the interface. But looking again it's got quite the verbose pointless implementation, and looking at the original commit it just looks like dead code.
WDYT?
There was a problem hiding this comment.
Wow, this is dead code indeed! Let's remove it!
|
👍 |
|
@bors r+ Thanks! |
|
📌 Commit 05e8194 has been approved by |
Try & simplify Registry vs Source confusion Refs #3006. I tried to go further and replace references of `Registry` with `PackageRegistry`, but I ran into lifetime issue in `ops::resolve_with_previous` which I think has to do with no longer passing `PackageRegistry` as a `Registry` trait object (with a new and distinct lifetime).
|
☀️ Test successful - status-appveyor, status-travis |
Refs #3006.
I tried to go further and replace references of
RegistrywithPackageRegistry, but I ran into lifetime issue inops::resolve_with_previouswhich I think has to do with no longer passingPackageRegistryas aRegistrytrait object (with a new and distinct lifetime).