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

Replaced status code number with constants #1924

Merged
merged 1 commit into from
Feb 6, 2015
Merged

Replaced status code number with constants #1924

merged 1 commit into from
Feb 6, 2015

Conversation

ajaybhargavb
Copy link
Contributor

rynowak
rynowak reviewed Jan 30, 2015
View reviewed changes
src/Microsoft.AspNet.Mvc.Core/ActionResults/CreatedAtActionResult.cs
@@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using Microsoft.AspNet.Mvc.Core;
using Microsoft.AspNet.WebUtilities;
Copy link
Member

Choose a reason for hiding this comment

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

@Tratcher would you guys consider moving this to more common namespace? As is it kinda makes me sad 😢

Copy link
Member

Choose a reason for hiding this comment

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

With or without moving it to a new assembly?

Suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

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

Microsoft.aspnet.http ?

Copy link
Member

Choose a reason for hiding this comment

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

shrug File a bug. HttpAbstractions needs a full organizational review anyways.

Copy link
Member

Choose a reason for hiding this comment

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

aspnet/HttpAbstractions#182

@rynowak
Copy link
Member

rynowak commented Jan 30, 2015

Take a look at BadRequestErrorMessageResult and also ApiController. I think some stuff in webapi shim got missed.
⌚

@yishaigalatzer
Copy link
Contributor

:shipit: when @rynowak is happy

@ajaybhargavb
Copy link
Contributor Author

@rynowak, Should the System.Net HttpStatusCode also be replaced?

dougbu
dougbu reviewed Jan 30, 2015
View reviewed changes
test/Microsoft.AspNet.Mvc.Core.Test/ActionResults/BadRequestObjectResultTests.cs
@@ -3,6 +3,7 @@

using Xunit;
using Microsoft.AspNet.Mvc.ModelBinding;
using Microsoft.AspNet.WebUtilities;
Copy link
Contributor

Choose a reason for hiding this comment

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

sort usings

@dougbu
Copy link
Contributor

dougbu commented Jan 30, 2015

:shipit: when @rynowak is happy and you've cleaned up the couple of out-of-order using additions

@ajaybhargavb
Copy link
Contributor Author

Updated. Replaced System.Net.HttpStatusCode usages.

@rynowak
Copy link
Member

rynowak commented Feb 3, 2015

thought the decision was to get rid of HttpStatusCode use except in the functional tests? the functional tests are HttpClient apps and are already playing in the System.Net / System.Net.Http pool.

I had the same feedback (leave the functional test code alone) and I'm equally confused. Let's discuss tomorrow

@rynowak
Copy link
Member

rynowak commented Feb 6, 2015

:shipit:

@ajaybhargavb ajaybhargavb merged commit ac6a1a6 into dev Feb 6, 2015
@ajaybhargavb ajaybhargavb deleted the replace-statuscodes branch April 23, 2015 22:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ajaybhargavb @rynowak @yishaigalatzer @dougbu @Tratcher