Have gix-filter depend on gix-packetline-blocking unaliased
#1940
Conversation
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
This removes the `gix-packetline-blocking/futures-lite` "ghost feature" added in d5dd239 (GitoxideLabs#1939), and instead: - Changes `gix-filter` to depends on `gix-packetline-blocking` without aliasing it `gix-packetline`. - Replaces uses of `gix_packetline` with `gix_packetline_blocking` in the code of `gix-filter`. Thus, this replaces the solution in GitoxideLabs#1939 for the problem discussed in GitoxideLabs#1929 (comment) with a different solution that avoids carrying a "ghost feature." In contrast, the long-standing `gix-packetline-blocking/async-io` feature added in be4de0d (GitoxideLabs#1123), though it should not be used, is *not* removed, because: - It is referenced in attributes in `gix-packetline-blocking` code (because `gix-packetline-blocking/src` is copied from `gix-packetline/src` via `etc/copy-packetline.sh`). - For that reason, removing it would cause the `clippy` run in the CI `lint` job to fail, as well as likely making various other reasonable operations fail.
|
Thanks so much, you are on fire!
Indeed, this was only for convenience, but given that it needs the blocking implementation, the code may as well reflect it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 removes the
gix-packetline-blocking/futures-lite"ghost feature" added in d5dd239 (#1939), and instead:gix-filterto depends ongix-packetline-blockingwithout aliasing itgix-packetline.gix_packetlinewithgix_packetline_blockingin the code ofgix-filter.Thus, this replaces the solution in #1939 for the problem discussed in #1929 (comment) with a different solution that avoids carrying a "ghost feature."
In contrast, the long-standing
gix-packetline-blocking/async-iofeature added in be4de0d (#1123), though it should not be used, is not removed, because:gix-packetline-blockingcode (becausegix-packetline-blocking/srcis copied fromgix-packetline/srcviaetc/copy-packetline.sh).clippyrun in the CIlintjob to fail, as well as likely making various other reasonable operations fail.This PR proposes the alternative alluded to in #1939. As they are described in #1929 (comment), this replaces way (1) with way (2). At the time it seemed like combining them, i.e. way (3), could make sense, but I think the phantom nature of the feature, together with how
gix-filterseems to be the only user ofgix-packetline-blocking, indicates that it shouldn't be carried if it's not actually being used.The reason I didn't go with this initially is that I am not sure I understand why
gix-packetline-blockingwas aliasedgix-packetlinewhen given as a dependency ofgix-filter. If it was solely for the convenience of being able to writegix_packetlineinstead ofgix_packetline_blocking, then I think the change here is an improvement. If there are deeper reasons, however, then I am not sure.I think this could be done with
use gix_packetline_blocking as gix_packetline;, but it didn't look like it would be better: it would have to be written in multiple places in the same file, due to the scope ofusedeclarations in files that define multiple nested modules. (And maybe also related to special semantics for#[from]? But I am not sure about that.)