Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to take arbitrary socket instead of address to estaplish connexion to proxy #20

Merged
merged 10 commits into from
Jul 15, 2020

Conversation

trinity-1686a
Copy link
Contributor

This is an attempt at solving both #5 and #19.
Most code is here, but it lacks documentation, tests, and an example.

This does break compatibility as some types that were fixed are now parametrized, however I've mostly kept previous interface so that changes are actually minimal.

@trinity-1686a trinity-1686a marked this pull request as ready for review July 11, 2020 20:09
@trinity-1686a
Copy link
Contributor Author

I think this is ready, but the CI is failing and I can't figure out why. Any insight is welcome.
Also if you have some comments about something not being like you'd like, don't hesitate

Copy link
Owner

@sticnarf sticnarf left a comment

Choose a reason for hiding this comment

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

The code looks good to me. But you'd better reformat the project with the nightly toolchain (some settings in .rustfmt.toml are not available on stable).

I think the CI failure is caused by concurrent tests. --test-threads 1 may help. You can add it to tests/integration_tests.sh and try again.

src/tcp.rs Outdated Show resolved Hide resolved
Copy link
Owner

@sticnarf sticnarf left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks very much!

@sticnarf sticnarf merged commit 55f95ad into sticnarf:master Jul 15, 2020
@trinity-1686a trinity-1686a deleted the generic-socket branch July 15, 2020 22:19
@sticnarf sticnarf mentioned this pull request Jul 16, 2020
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