Skip to content

Conversation

@slobo80
Copy link

@slobo80 slobo80 commented Apr 14, 2017

The example incorrectly is invoking Add( method instead of SetCookie

Title

On the title describe
what you've fixed (or created) with this Pull Request (PR).

Summary

Insert a short (one or two sentence) summary here.

Fixes #Issue_Number

Note: The "Fixes #nnn" syntax in the PR description causes
GitHub to automatically close the issue when this PR is merged.
Remove that line if you don't have issues associated with this
PR. Click on the Guidelines for Contributing link above for details.

Details

Explain your changes, and why you made them. If that
information is already available in the issue referenced
above, just referencing the issue is preferred to copying
the text.

This may not be necessary depending on the scope of the PR
changes. For example, "fix typo in introduction.md" is
sufficient to describe that PR.

Suggested Reviewers

If you know who should review this, use '@' to request a review.

The example incorrectly is invoking Add( method instead of SetCookie
@dnfclas
Copy link

dnfclas commented Apr 14, 2017

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@rpetrusha
Copy link
Contributor

Thanks for pointing to this inconsistency, @slobo80. As your PR #1917 notes, though, HttpResponse.SetCookie is for internal use only. Given that, we don't want to provide an example that calls the internal-only member.
Rather than modifying the code, would you mind revising the comment that precedes the code? It can read something like, "Because the HttpResponse.SetCookie method is intended for internal use only, you should not call it in your code. Instead, you can all the HttpResponse.Cookies.Add method, as the following example shows." Thanks.

Copy link
Contributor

@rpetrusha rpetrusha left a comment

Choose a reason for hiding this comment

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

Changes requested in previous comment.

@guardrex
Copy link
Contributor

@rpetrusha
Copy link
Contributor

Sorry, @slobo80, I looked at the C# code but not the VB code. Would you mind changing the VB code as well to call Add instead of SetCookie? If that's too much trouble, let us know, and we can close this PR and open one ourselves.

@slobo80
Copy link
Author

slobo80 commented Apr 15, 2017

Submitted a new PR for the VB code.

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.

4 participants