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

Add UnsafeAuthenticatedConnectionSharing flag to RestClient #764

Merged
merged 1 commit into from Dec 22, 2017

Conversation

oleveau
Copy link
Contributor

@oleveau oleveau commented Oct 9, 2015

Add UnsafeAuthenticatedConnectionSharing flag to RestClient so that we
can set it to the underlying webrequest.

Add UnsafeAuthenticatedConnectionSharing flag to RestClient so that we
can set it to the underlying webrequest.
@hallem
Copy link
Member

hallem commented Apr 26, 2016

The changes look good. Can you fix the merge conflicts and add some unit tests?

/// <summary>
/// Flag to send authorisation header with the HttpWebRequest
/// </summary>
public bool UseUnsafeConnection { get; set; }
Copy link

Choose a reason for hiding this comment

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

unused?

Copy link
Member

Choose a reason for hiding this comment

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

@KOTRET could you elaborate?

Copy link

Choose a reason for hiding this comment

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

@alexeyzimarev prop UnsafeAuthenticatedConnectionSharing (Line 203) is definitely in use, but i cannot find usage of the prop UseUnsafeConnection (line 208) , which is also added with this commit.

Copy link
Member

Choose a reason for hiding this comment

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

This is a public API in a library. "Unused" in this context makes little sense. Most of the public API methods are "unused" inside the library, they are public because people who download the library will use it.
I agree in a sense that if it is "unused" - it means there are no tests for it. This actually is a problem.

Copy link
Member

Choose a reason for hiding this comment

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

@KOTRET I see what you mean. It is indeed added without any usage.

@alexeyzimarev alexeyzimarev changed the base branch from master to develop November 18, 2017 14:05
@alexeyzimarev alexeyzimarev merged commit 5b65d30 into restsharp:develop Dec 22, 2017
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.

None yet

4 participants