Skip to content
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

Proposal for new Yesod project skeleton structure #997

Closed
s9gf4ult opened this issue May 26, 2015 · 17 comments
Closed

Proposal for new Yesod project skeleton structure #997

s9gf4ult opened this issue May 26, 2015 · 17 comments

Comments

@s9gf4ult
Copy link
Contributor

Hello, here is a proposal for improovement of structure of new (scaffolded) Yesod project.

https://github.com/s9gf4ult/yesod-skel

What do you think about?

@gregwebs
Copy link
Member

This looks well thought out, thank you!

On the whole it looks good, and I have used some similar techniques in the past to make App available outside the Handler.

I don't like the name Helper because it is so generic as to be meaningless. If the commonality is access to App, then perhaps it should be put under the namespace App instead of Helper. I think your goal with the Helper namespace is to write code that can be used in a Handler or used elsewhere (a background job).

What I have always done for separating code is to place all of the code for database queries into Model/*. That code only operates in the database monad and does not need App. That makes testing the database interaction easy and provides good code organization. I would like to see this change introduced into the scaffold itself to start users off on a more organized approach.

@s9gf4ult
Copy link
Contributor Author

If the commonality is access to App, then perhaps it should be put under the namespace App instead of Helper

No, the commonality is that code does not depend on App at all, and this code can be accessed from anywhere including Foundation.

. I think your goal with the Helper namespace is to write code that can be used in a Handler or used elsewhere

Yes, it is.

place all of the code for database queries into Model/*.

Yes, this is also reasonable, but what I mean under Helper is more generic, not just working with DB but authentication, authorization, form/json parsing/generating, any other logic wich required to be in Handler monad.

One guy offered next structure of project:

App.FoundationImport  -- should be imported by `*.Handler` modules only
App.NoFoundationImport -- should be imported by any other module which must not be dependent from `App`
App.DB.* -- for models and working with database
App.Route.Home.Handler -- same as `Handler.Home` in old structure. This module depends on `Foundation` and have access to `Route App` (import App.FoundationImport)
App.Route.Home.Form -- for form types which is used in `App.Route.Home.Handler`. This module written in independent from `App` fassion (import App.NoFoundationImport)
App.Route.Home.Common -- some other code used in elsewhere in Form or Handler (import App.NoFoundationImport)
App.Common.* -- Some other common modules used anywhere in project. Should not depend on `App` and `import App.NoFoundation`

@MaxGabriel
Copy link
Member

I really like this.

@dpwiz
Copy link
Contributor

dpwiz commented May 26, 2015

I really like how the code is packed neatly under src/ and not spilling to almost every directory in a project.

@snoyberg
Copy link
Member

I'm definitely OK with changes along these lines. Ultimately this should become a PR on the yesod-scaffold repository's postgres branch in order to replace the current scaffolding.

Has this been tested with yesod devel?

@s9gf4ult
Copy link
Contributor Author

tested with yesod devel?

No, I did not.

I would PR this, but I have no idea how to pack this skeleton into project-template's template file.

@snoyberg
Copy link
Member

That's what I was saying about yesod-scaffold; have a look at https://github.com/yesodweb/yesod-scaffold#readme

@s9gf4ult
Copy link
Contributor Author

Oh, looks like I failed. I was must fork yesod-scaffold instead of creating yesod-skel from the scratch.

@s9gf4ult
Copy link
Contributor Author

I have just updated yesod-skel. Is this ok to PR in yesod-scaffold this project structure?

@snoyberg
Copy link
Member

It's certainly OK to PR, no objection there. @gregwebs or I may have more comments before it can be merged, however.

@gregwebs
Copy link
Member

I will take a closer look at things tomorrow. I would still like to come up with a better name than Helper if possible.

@s9gf4ult
Copy link
Contributor Author

I have removed Helper in favour of App namespace and different project structure. It is already in yesod-skel.

@s9gf4ult
Copy link
Contributor Author

Pull request created

@cies
Copy link

cies commented Jul 24, 2015

Really great initiative!

I have a few questions, and some remarks.

  • The term "app" is too much overloaded: app/, Application, App. To make it slightly less: app/ could be main/ (app/ is also a misleading for those from a Rails(ish) background).
  • Could we rename DevelMain.hs => GhciMain.hs, main.hs => Main.hs and devel.hs => Devel.hs?
  • I think routes does not belong in config. I think it belongs more in src/. In config/ I expect stuff that is not compiled into my app.
  • I think favicon.ico and robots.txt belong in static/ and not in config/.
  • Can we have a db/ folder where some db related things go, like sqlite-files, or the occasional migration script.
  • We could put a line in the cabal-file dependency list and suggest to put additional dependencies under the line; this makes is easier to compare the deps-from-skeleton at a later point, i.e.: when upgrading the app to a new LTS.
  • Settings.hs and the Settings namespace could be renamed to Config or Configuration (I favor the latter as it will look cool next to Applica_tion_ and Founda_tion_). Usually in Haskell namespaces are not plural. Also "settings" sound to me like some basic values, but this contains code! Also, would it be possible to split the file now called Settings.hs up into several files under the now called Settings/, each one on a topic (and re-export them)?
  • I feel the Route namespace does not really cover the contents. Especially given Common, forms, but no routing are in there. I feel that forms may be better off close to App.Model (if any), or in their own App.Form namespace. Another name for Route could be Controller or back to Handler (now that the forms are out it will not get too crowded in there for up to medium sized apps).
  • For importing handlers, could we not have something like a generic version of the hspec-autodiscover preprocessor to auto-add them (this would still need cabal file changes, which could at some day maybe also be avoided with the introduction of wildcards)?
  • Handler.hs could be renamed to HandlerImport.hs, more descriptive and in line with the other *Import.hses.
  • The App.DB from the README is something that I would definitely call App.Model. Is that also the place where the model file (TH) would go? I think a place for functions that are close to the model is needed; like specifying some toJSON instance or more complex queries; I'd like to put all that in App.Model.

Wow. That is quite a list! Please to not take my comments as something against your work: I strongly agree that revising the structure is worth it, and I really like what you are showing in your proposal.

But changing things on this matter it is is not cheap: docs go out of sync, StackEx answers go out of sync, some people will want to update their apps to reflect the new structure. I think for that matter is it important to do it "all the way" (maybe there are also some function/datatype names that we want to improve upon), together with a major version bump, and then not touch it for a while.

I really hope this will make Yesod projects more accessible, especially to newcomers.

@gregwebs
Copy link
Member

Thanks for chiming in. With respect to App, I started working at Front Row Education, and we have multiple apps. So there is actually some potential benefit to namespacing with the actually application name (project name given at initialization time) rather than App: stack ghci could better deal with loading multiple apps.

@cies
Copy link

cies commented Jul 24, 2015

@gregwebs iirc in rails they also started scoping to a custom name. I think the reason there was "in case you ever want to mount your app into an other app".

@jezen
Copy link
Member

jezen commented Jan 26, 2020

Discussion for this issue migrated to the above linked PR, so I will close this.

@jezen jezen closed this as completed Jan 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants