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

Added Cookie Feature. Now you can use Cookies with this restClient. #16

Closed
wants to merge 1 commit into from

Conversation

chfo
Copy link

@chfo chfo commented Jan 20, 2015

With this simple expansion the Rest Client is able to use Cookies. It saves the cookies to a file 'cookie' and use it with every following request. This can be used for a simple authentification check, which should be part of a restfull service.

@mrtazz
Copy link
Owner

mrtazz commented Jan 20, 2015

This is awesome! Thanks for the contribution! Unfortunately it seems that this patch breaks the tests. Could you look into that? It would also be great to have unit tests for this new functionality. I was looking into changing over the tests to use http://httpbin.org anyways, so you can totally use that as a source for integration data.

@chfo
Copy link
Author

chfo commented Jan 20, 2015

Hi. I will try to fix this and give you some unit tests.

@chfo
Copy link
Author

chfo commented Jan 21, 2015

Hi,
I think that the test is not passing is not a problem with the new code. I downloaded the 'old' code and tested it with a minimal example:
RestClient::response r;
r = RestClient::get("http://http-test-server.heroku.com");
cout << r.code << endl;
I still get Code 301.

After that i tested it in my terminal:
[chris@Poro ~]$ curl -I http://http-test-server.heroku.com
HTTP/1.1 301 Moved Permanently
Connection: keep-alive
Server: Cowboy
Date: Wed, 21 Jan 2015 11:30:02 GMT
Content-Length: 0
Location: http://http-test-server.herokuapp.com/

Still Code 301.

//The master branch test is still failing too.

Best regards

@mrtazz
Copy link
Owner

mrtazz commented Aug 9, 2015

@chfo sorry for the super long delay on this. I just fixed the tests in master, would you mind merging it in so we can have a proper build status here? Unfortunately I haven't yet added a way to easily parse the resulting JSON but have opened #23 for this

@mrtazz
Copy link
Owner

mrtazz commented Aug 14, 2015

the current unit test suite in master now also supports JSON parsing (#25)

@mrtazz mrtazz added this to the v0.3.0 milestone Sep 3, 2015
@mrtazz
Copy link
Owner

mrtazz commented Dec 26, 2015

@chfo do you have time to make those adaptions so this can be merged?

@mrtazz mrtazz removed this from the v0.3.0 milestone Jan 10, 2016
@mrtazz
Copy link
Owner

mrtazz commented Feb 2, 2016

This won't apply to master anymore. I'd still be interested in this functionality if you have the time to base it off the current design. Closing this in the meantime.

@mrtazz mrtazz closed this Feb 2, 2016
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.

3 participants