Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Added an overload for StatusCode which takes a HttpStatusCode paramet…#6902

Merged
javiercn merged 2 commits intoaspnet:devfrom
dale-palmer:patch-1
Oct 26, 2017
Merged

Added an overload for StatusCode which takes a HttpStatusCode paramet…#6902
javiercn merged 2 commits intoaspnet:devfrom
dale-palmer:patch-1

Conversation

@dale-palmer
Copy link
Contributor

Now takes a HttpStatusCode parameter as well as int.

@dnfclas
Copy link

dnfclas commented Oct 2, 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

/// <returns>The created <see cref="StatusCodeResult"/> object for the response.</returns>
[NonAction]
public virtual StatusCodeResult StatusCode(HttpStatusCode statusCode)
=> new StatusCode((int)statusCode);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't have new?

error CS0246: The type or namespace name 'StatusCode' could not be found (are you missing a using directive or an assembly reference?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this doesn't look quite right. I agree the new needs to be removed because it's just chaining a method call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops! Sorry, wrote it without a compiler based on something else I was working on. You're absolutely right.

/// <returns>The created <see cref="StatusCodeResult"/> object for the response.</returns>
[NonAction]
public virtual StatusCodeResult StatusCode(HttpStatusCode statusCode)
=> new StatusCode((int)statusCode);
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this doesn't look quite right. I agree the new needs to be removed because it's just chaining a method call.

@dale-palmer
Copy link
Contributor Author

Amendment made.

Copy link
Contributor

@Eilon Eilon left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@javiercn - can you review & merge?

/// <param name="statusCode">The status code to set on the response.</param>
/// <returns>The created <see cref="StatusCodeResult"/> object for the response.</returns>
[NonAction]
public virtual StatusCodeResult StatusCode(HttpStatusCode statusCode)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also have a public virtual ObjectResult StatusCode(HttpStatusCode statusCode, object value) overload to mirror the int overload?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine too.

Copy link
Member

Choose a reason for hiding this comment

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

I'll file a bug and make it up for grabs

@javiercn javiercn merged commit c567a69 into aspnet:dev Oct 26, 2017
@dale-palmer dale-palmer deleted the patch-1 branch October 27, 2017 13:56
kichalla added a commit that referenced this pull request Apr 27, 2018
…System.Net.HttpStatusCode"

This reverts commit c567a69.

[Fixes #7709] Revert #6902
kichalla added a commit that referenced this pull request May 1, 2018
…System.Net.HttpStatusCode"

This reverts commit c567a69.

[Fixes #7709] Revert #6902
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants