amber icon indicating copy to clipboard operation
amber copied to clipboard

Published 20 hours ago • verified publisher icon amberframework

Feature/change default orm to jennifer

Open crimson-knight opened this issue 1 year ago • 4 comments

Description of the Change

This PR changes the default ORM to Jennifer, and integrates the CLI commands for Jennifer into the amber CLI tool.

A couple of nice changes:

  • Detects the version of node and updates the node-sass dependency versions so that styling properly compiles if you're using Node 14+
  • Changed the 3rd link in the auto generated Amber app to link to the discord
  • Adds the Sam CLI utility to make running tasks easy.

Potential breaking change:

  • The I18n shard that Jennifer relies on removes the force_locale: context.locale feature. I think this means we no longer have "dynamic" language detection. I don't see any tests currently for this, but it's worth trying to add this back in and I think using URL based language detection. The default language can be configured easily, and all the translation capabilities work it is just not dynamic.

Benefits

Not having to solo-maintain an ORM and using an ORM that has more of the db-specific data types working (mostly looking at Postgres)

Possible Drawbacks

Currently, this tightly couples the Amber CLI tool with the Sam CLI tool provided by Jennifer. The Sam tool is pretty handy for rake/make-like jobs, so I'm pretty happy with doing this. I think there will be a lot of situations where other devs will want to run tasks (data migrations/alterations, etc). so this will be a handy improvement.

crimson-knight avatar Jul 12 '22 13:07 crimson-knight

Hi Seth. Thanks for submitting this PR. This is a very large PR and will take some time to review. I will give it a spin when I get a chance. Will you be able to update the documentation as well?

drujensen avatar Jul 13 '22 01:07 drujensen

Also, looks like the Dockerfile is still using crystal 1.0.0 and doesn't build.

drujensen avatar Jul 13 '22 02:07 drujensen

Thanks @drujensen for the review, you're right I haven't update the auth generator yet. I had forgotten about the docker file, but I will need to update that in a different PR since that's a different scope from this one.

I'll definitely get the documentation updated once this PR is done and I know the full extent of the changes.

crimson-knight avatar Jul 13 '22 09:07 crimson-knight

Oh yeah, FWIW a lot of the file changes are just tweaks suggested by Ameba so I could get the bin/amber_spec to pass.

crimson-knight avatar Jul 13 '22 10:07 crimson-knight

Closing this out in favor of a different approach that allows for any ORM to be used and connected to the generators/CLI tool

crimson-knight avatar Oct 31 '22 16:10 crimson-knight