Split resolver module to make it more manageable #5258
Conversation
|
✌️ @Eh2406 can now approve this pull request |
|
I love the Iday! Thank you for doing this. Is there anything in particular I should look out for in reviewing? |
|
📌 Commit 7b9050d has been approved by |
|
⌛ Testing commit 7b9050d027026e5de1540e33fbfcad1f0b1ae0a2 with merge c8551e0ff2ce275ef00d9b80c15ecf8bcfb577b5... |
|
💔 Test failed - status-travis |
|
Looked it over, assuming that the |
|
I will approve when CI is green. Thanks for doing this! |
|
(Note that Travis won't go green for the |
|
Ya, just figured that out. |
|
📌 Commit d0bedf0 has been approved by |
|
💡 This pull request was already approved, no need to approve it again. |
|
📌 Commit d0bedf0 has been approved by |
|
⌛ Testing commit d0bedf0 with merge be67876921be5956455be24d6f723e83918cc313... |
|
💔 Test failed - status-travis |
|
Do you want to try a manual merge or a rebase? |
|
opps, ya that can happen. If you dell the branch while bors is testing then you cancel the merge. |
|
Once #5261 is finally merged, I'll rebase this on top of master again. |
Fix formatting issues with cargo fmt Noticed these unrelated formatting problems while working on #5258.
|
@bors r+ |
|
💡 This pull request was already approved, no need to approve it again.
|
|
📌 Commit d0bedf0 has been approved by |
Split resolver module to make it more manageable The `core::resolver` module is currently the largest in Cargo, at some 2000 lines. Here's an attempt at splitting it into more reasonable parts and reordering the code in it for better comprehensibility. Splitting is done in three major steps: * Move the `Resolve` type and its dependencies into a separate module (~220 lines) * Move utility data types into a separate module (~400 lines) * Move the `Context` type and its dependencies into a separate module (~400 lines) This halves the size of the root module, which is then reordered to make it more readable. (This is a yak-shaving expedition of sorts, in preparation for #1286.)
|
One more time: |
|
📌 Commit 45da574 has been approved by |
Split resolver module to make it more manageable The `core::resolver` module is currently the largest in Cargo, at some 2000 lines. Here's an attempt at splitting it into more reasonable parts and reordering the code in it for better comprehensibility. Splitting is done in three major steps: * Move the `Resolve` type and its dependencies into a separate module (~220 lines) * Move utility data types into a separate module (~400 lines) * Move the `Context` type and its dependencies into a separate module (~400 lines) This halves the size of the root module, which is then reordered to make it more readable. (This is a yak-shaving expedition of sorts, in preparation for #1286.)
|
☀️ Test successful - status-appveyor, status-travis |
Introduce FeatureValue type to represent features table values This is the next step towards #1286 (after #5258). The goal here is to have a central place in the code where feature strings are interpreted as (a) a feature, (b) a dependency or (c) a dependency/feature combo, and anchor that interpretation in the type system as an enum. I've spent quite a bit of effort avoiding extra string allocations, complicating the code a bit; notice in particular the use of `Cow<str>` in `FeatureValue` variants, and the slight workaround in `Context::resolve_features()` and `build_requirements()`. I hope this is all okay. cc @Eh2406
The
core::resolvermodule is currently the largest in Cargo, at some 2000 lines. Here's an attempt at splitting it into more reasonable parts and reordering the code in it for better comprehensibility.Splitting is done in three major steps:
Resolvetype and its dependencies into a separate module (~220 lines)Contexttype and its dependencies into a separate module (~400 lines)This halves the size of the root module, which is then reordered to make it more readable.
(This is a yak-shaving expedition of sorts, in preparation for #1286.)