-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Notes on FirstApp #14
Comments
Thanks for all the feedback. It all sounds like great ideas. When I get back from vacation I'll definitely take a look and implementing all of it. We don't have uri_part by default, because not all of our apps have a web component. Many are purely REST, and the uri_part is unneeded then. That said, "wing class --add=MyClass" does automatically add the role. |
Which is fair enough, I'm just thinking generating ->id and then having to edit it in the generated web code seems silly compared to having 'sub uri_part { shift->id }' composed in when you -are- doing web stuff so you can generate dancer code that uses uri_part to start with. |
Oh, I see now. You don't actually have to do that editing part. The docs are out of date. I'll get that fixed too. "wing class --add=MyClass" and "wing class --template=MyClass" are automatically using the uri_part these days. |
Aha. I just read through the FirstApp.pod, hence the title of the issue. Should've put that in the body as well, sorry. |
For things that you always want importing, you might consider Import::Into / Import::Base to bundle them together to avoid the boilerplate.
Might also be nice to use exported subroutines instead of class method calls for the DBIC stuff plus DBIx::Class::Candy for declaring DBIC specific bits.
search()->single is vomit worthy, instead declare a unique constraint in your result class, which will make DBIC deploy the right thing, and you'll be able to use find().
also it seems strange to me not to have the uri_part by default but using 'id' by default, and then you could auto-generate routes that already have the correct redirect (plus I'd abstract out the redirect generation a bit but I guess that's a taste thing)
The text was updated successfully, but these errors were encountered: