Use Mvc.Core instead of Mvc
#300
Conversation
|
There appears to be one failing test. Until #290 is completed, I won't be able to figure out which one is failing, since output is extremely verbose (and unhelpful). Otherwise, maybe someone can point out which test is failing? |
There was a problem hiding this comment.
Looks great! Thanks for doing this. The one thing I need to figure out is what version to release this at.
For debugging test failure, you can go to Appveyor directly.
I'm not seeing what part of this PR is causing it to fail immediately, so I may need to dig into it a little later. But overall this looks really good.
| @@ -4,28 +4,27 @@ | |||
| <NetCoreAppVersion>netcoreapp2.0</NetCoreAppVersion> | |||
| <NetStandardVersion>netstandard2.0</NetStandardVersion> | |||
|
|
|||
| <AspNetCoreVersion>2.0.1</AspNetCoreVersion> | |||
| <AspNetCoreVersion>2.1.0</AspNetCoreVersion> | |||
There was a problem hiding this comment.
I'm a bit nervous about pushing this in a non-major release until #281 is done
There was a problem hiding this comment.
We could get #281 and bump the version or just release it. I suspect there will be no significant issues since 2.0 to 2.1 is more of a performance upgrade so just releasing might be fine.
There was a problem hiding this comment.
fyi - I've started this work over in #302 , should have it done tonight or tomorrow....the fix for the failing tests here will be the same as in that PR.
| @@ -5,7 +5,7 @@ namespace JsonApiDotNetCoreExample.Controllers.Restricted | |||
| { | |||
| [Route("[controller]")] | |||
| [HttpReadOnly] | |||
| public class ReadOnlyController : Controller | |||
| public class ReadOnlyController : ControllerBase | |||
There was a problem hiding this comment.
Thank you! I've been wanting to do this for a while. I don't think anyone is generating views via their JADNC controllers, but if they are this would be a breaking change 🤔 ... I think this is the right direction. Just wondering about versioning.
| @@ -11,7 +11,7 @@ public class CustomErrorTests | |||
| public void Can_Return_Custom_Error_Types() | |||
| { | |||
| // arrange | |||
| var error = new CustomError("507", "title", "detail", "custom"); | |||
| var error = new CustomError(507, "title", "detail", "custom"); | |||
| @@ -131,7 +131,7 @@ public AttributeFilterTests(TestFixture<TestStartup> fixture) | |||
|
|
|||
| // assert | |||
| Assert.Equal(HttpStatusCode.OK, response.StatusCode); | |||
| Assert.False(deserializedTodoItems.Any(i => i.Ordinal == todoItem.Ordinal)); | |||
| Assert.DoesNotContain(deserializedTodoItems, x => x.Ordinal == todoItem.Ordinal); | |||
|
|
||
| <EFCoreVersion>2.0.1</EFCoreVersion> | ||
| <EFCoreToolsVersion>2.0.1</EFCoreToolsVersion> | ||
| <EFCoreVersion>2.1.0</EFCoreVersion> |
There was a problem hiding this comment.
The test is failing because of this. We do some reflection on EF APIs to check the actual SQL output of queries (ToSql). Looks like these properties no longer exist or were renamed.
There was a problem hiding this comment.
#302 has a fix for this and includes the tests for the prior frameworks. I'll probably merge that in tonight and then you can rebase onto those changes.
There was a problem hiding this comment.
I decided to hold on #302 for now. I'm going to merge this in and get a release started.
|
Thanks @jaredcnance! |
Upgrade to AspNetCore/EfCore 2.1 and use `Mvc.Core` instead of `Mvc`
I wanted to add the package to my project and it asked to install a lot of dependencies. First step to reduce that is to use
Mvc.Coreinstead ofMvc.Mvchas a lot of extra unneeded packages, such as Razor (template support).I also updated all other packages.
I noticed a lot of warnings related to
xunit. Probably existing, but I fixed those as well.