sugar-framework / sugar

Modular web framework for Elixir
https://sugar-framework.github.io/
MIT License
430 stars 29 forks source link

Upgrade to Ecto 0.7.1 and Postgrex 0.7 #63

Closed YellowApple closed 9 years ago

YellowApple commented 9 years ago

Now the multitude of Ecto features, bugfixes, etc. introduced since 0.2.x are available for use in Sugar.

This includes a tweak to Mix.Tasks.Sugar.Init to pass an extra "-r" before the name of the to-be-generated repo due to a syntax change on Ecto's end. Otherwise, haven't encountered any compatibility issues thus far (nor should there be any, I reckon).

YellowApple commented 9 years ago

Darn, tests are failing due to a difference in Ecto's repo generator output. One moment while I see if I can get them to pass again...

slogsdon commented 9 years ago

It looks like the build failure is coming from this line not having the :otp_app key that was recently introduced. To be honest, I'm not sure what function that module was even serving, so it may be able to be removed from the test module. I can dig into that tonight to see what's going on with it.

YellowApple commented 9 years ago

Yeah. I've dug into the model and scaffold tests and cleaned that up a bit (added otp_app: :test_app to the Ecto.Repo arguments, then cleaned out the old method signature that was no longer in use by Ecto), and I feel like I'm getting closer to a passing test suite, though it's still failing due to some environment issues (namely, Ecto not recognizing :test_app as a valid OTP application name).

The Repos.Main module in those tests is required in order for Ecto's generators to run (since they otherwise will complain that a valid OTP application hasn't been specified. So I guess the trick is to fool Ecto into thinking that :test_app is a valid application.

YellowApple commented 9 years ago

Alright, that last commit should fix the tests, but at the cost of a dirty hack: pass :sugar for the :otp_app value Ecto expects and forcibly set a configuration value (with Application.put_env(:sugar, Mix.Tasks.Sugar.ScaffoldTest.Repos.Main, []) in the scaffold test, and similar for the model test). Probably not an elegant solution by any means, but hey, if Ecto insists on having a valid OTP app, then we might as well give it one.

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 94.46% when pulling 284f1e008f3706331b9e90a7ecd3a5d37a574ff1 on YellowApple:master into e638a306e330509f9507c00f95389d33f67afcc6 on sugar-framework:master.

slogsdon commented 9 years ago

Thanks, @YellowApple! This works for me. We can always revisit to clean things up if need be.

BTW, interested in collaboration privileges? Your work is good, and if you're going to be submitting more of it, being a collaborator would definitely make things easier for you.

YellowApple commented 9 years ago

Sure!

slogsdon commented 9 years ago

Invite sent