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

Fix broken build and tests with --no-default-features #22

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jirutka
Copy link

@jirutka jirutka commented Oct 9, 2022

No description provided.

@jirutka jirutka force-pushed the fix-no-default-features branch from 319bd79 to 4a74cd9 Compare October 9, 2022 20:07
@iddm
Copy link
Owner

iddm commented Oct 17, 2022

I sincerely apologise for the late response. Many things are going on right now. I will have time to review either this week or the next week, unfortunately.

@iddm
Copy link
Owner

iddm commented Oct 21, 2022

Hi again! First of all, thank you for taking time to contribute! I have a little proposal for this pull request. As it already touches what I have wanted to change for quite some time now, perhaps, now it is a good time to do that, of course, if you don't mind. The thing is, the HeaderMap used in request.rs isn't really needed there in the structure called Request. What we need there is just something what may hold unique associations of strings. A good candidate, for example, would be the std::collections::HashMap. Can you please remove the http crate dependency for good and replace the code using it so that it uses just a hash map, and in case of the client feature being enabled, it would be creating a http::HeaderMap object on-the-fly? If you don't have time for this, I can just merge the code right now and do this myself, no problem at all.

In case you were to ask me why I wanted to do this, that is because I want the crate to have as minimal explicit dependencies as possible, so that anyone can use it anywhere as having it as a dependency (hopefully) wouldn't ever cause a dependency hell problem. This is also why the client feature is not enabled by default and is optional for the crate to work.

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