add todo sample project #50
Conversation
|
Awesome! I was looking into the possibility of using either git submodules or a subtree merge (the latter is the right answer I'm pretty sure) to bring an existing client implementation into the project. Of course now I can't find an ember data one out there...though I imagine it exists. |
|
Never mind, this one doesÖ https://github.com/tastejs/todomvc/tree/master/examples/emberjs . I thought it didnät use Ember Data when I looked at it a minute ago. |
|
It's too bad that is a globals-based app, I'd much rather have something using ember CLI. But I haven't used ember CLI in windows yet and from what I hear the experience is still less than ideal. Do you want me to try and incorporate that client app in this PR or do you think this is good to merge as-is? |
|
Yeah, I use a combination of require.js and ASP.NET bundling in my .NET Ember apps...I'm not sure it makes sense (yet?) to use ember-cli when you're in a VS environment. Maybe someday there'll be a NuGet package with a similar resolver? There are already a few Ember NuGets, but I'm not using any of them yet myself. Given that this is the "official" TodoMVC repo (I think) if we can use it I'd say lets do, regardless of the methodology. Anyone comparing will be used to looking at it like that. Nah, we can go ahead and merge this as the backend and add the frontend later. There's no milestone depending on this, it's a nice-to-have, so we can put it together piecemeal. |
|
Pushed a new diff, apparently I had broken the build right before checking in. |
|
Okay...I haven't really read up on OWIN before and now my head hurts (not the author's fault). I do have some questions, probably for my benefit, I'll comment in the code I think. |
| public void Configuration(IAppBuilder app) | ||
| { | ||
| // Setup db context for use in DI | ||
| app.Use(async (context, next) => |
There was a problem hiding this comment.
Would it be terribly non-standard to rename context to owinContext or appContext or something in this scope? When I first started reviewing this I came here and was completely turned around (not knowing OWIN) because I thought context was a DbContext (this was common naming practice in EF4 days...which was probably also bad).
There was a problem hiding this comment.
Sure, I'll rename it, sorry for the confusion.
|
A bit conflicted on this. I'm happy to have it, thank you, and we'll get it merged. But my hope was to be able to get the simplest possible example implementation built to show the value of the library. Trying to use this as a learning example would turn me off, personally--admittedly though largely because I don't grok OWIN. I think the way ahead is to do as you said and move a lot of the acceptance testing into JSONAPI.Tests instead. That will simplify the footprint substantially. Maybe going forward we can find a way to still apply modern techniques and best practices without having as many prerequisites to understanding the example though. |
|
Thanks for your feedback. One of my goals for this PR was to identify areas of the developer API surface we can streamline. Once this is merged we will have a central place to evaluate the impact of changes to that surface. Regarding OWIN, it is definitely the future for hosting web applications in the .NET world, and System.Web's days are numbered. Not using OWIN in our example at this juncture would be short-sighted. Yeah I'm now thinking that the acceptance testing for this project only serves to confuse the target audience of the example. We'd be better off moving those tests into the existing test project. I'll revise this diff to only include the sample project, and move the tests into a separate PR. |
Removed the todomvc tests project, and was able to simplify the example Startup code a bit.
|
Ok, I've pulled the acceptance tests out of here and moved them into #51. I was thus able to remove all the autofac stuff, so the Startup file is not as scary. Where should I put the documentation on how to use the sample app? Make a new wiki page? Or should we wait until we have a client app to go with it? |
|
Oh wow, yeah that's a million times better thanks! WRT Docs: Eh, we can probably wait till we can get the client app integrated. At least it's there to look at now. |
This implements the server-side component of #18. I figured that even without the client app, users will still benefit from having a reference implementation.
JSONAPI.TodoMVC.APIprovides an OWIN application using Web API with our formatter and filters, and controllers inheriting from the EF ApiController.JSONAPI.TodoMVC.API.Testsincludes some tests of theTodosControllerand more importantly, acceptance tests that involve establishing a server, making an HTTP request to that server, and evaluating the response. A lot of this testing methodology should probably be moved toJSONAPI.Testsand fleshed out more thoroughly there, but the TodoMVC app will need tests anyway so I figured here was as good a place as any to start.(Also,
JSONAPI.TodoMVC.APIis now the default startup project for the solution, so you can hit F5 and something useful will now run!)