Skip to content

Conversation

@djc
Copy link
Contributor

@djc djc commented Nov 1, 2025

Some miscellaneous stuff unrelated to transport implementations, and a bunch of stuff I missed because some async code was previously configured out if blocking code was enabled. Test that all features work in CI.

@djc djc force-pushed the transport-all branch 4 times, most recently from a336dfa to 564e885 Compare November 1, 2025 22:06
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much, and sorry for the delay!

The most notable change that I did was the renaming of Outcome::from_lines…() to Handshake, which seemed like a better fit.
Now I don't know if this is a mistake because it introduced ambiguity elsewhere, but if you encounter it, I am sure you'd be most qualified to find better names.


## If set, blocking implementations of the typical git transports become available in `crate::client::blocking_io`
##
## If used in conjunction with an async implementation, this one takes precedence.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might also be something to double-check @djc, I hope it's correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the async and blocking implementations are fully parallel -- I don't remember seeing any code where either one is prioritized, and I don't intend to introduce any such code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I fixed that in the other PR which is about to merge.

@Byron Byron enabled auto-merge November 4, 2025 07:01
@Byron Byron merged commit 7ce40ef into GitoxideLabs:main Nov 4, 2025
28 checks passed
@djc
Copy link
Contributor Author

djc commented Nov 4, 2025

The most notable change that I did was the renaming of Outcome::from_lines…() to Handshake, which seemed like a better fit.
Now I don't know if this is a mistake because it introduced ambiguity elsewhere, but if you encounter it, I am sure you'd be most qualified to find better names.

The Outcome name was pre-existing -- I didn't change it. I agree that it's not a great name, and Handshake is definitely an improvement. I think I saw some other types named Outcome as well...

@Byron
Copy link
Member

Byron commented Nov 4, 2025

I think I saw some other types named Outcome as well...

Yes, there are everywhere, and typically they really are nothing more than the outcome of some function. In the handshake case though, it made sense to name it, and in the other PR I also changed it, I think for the better. Please feel entitled to review these common names as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants