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

Add optional lucky extensions for avram #772

Merged
merged 6 commits into from
May 1, 2022

Conversation

matthewmcgarvey
Copy link
Member

@matthewmcgarvey matthewmcgarvey commented Dec 24, 2021

This is where the code that combines Lucky and Avram will live that was taken from luckyframework/lucky#1620.

To get the same behavior in apps as there was before, requires will be changed.

require "lucky"

becomes

require "lucky"
require "avram"
require "avram/lucky"

That should be the only breaking change. The tasks should continue to work, but the ones removed from lucky will no longer be preloaded (lucky gen.model and lucky gen.resource.browser)

I organized the code in a way that Avram can be run without the lucky dependency if desired. For instance, lucky is not included as a dependency of Avram (it is a development dependency, though). So, in order to use require "avram/lucky", you must add lucky as a dependency or use in a lucky app.

@jwoertink jwoertink marked this pull request as ready for review May 1, 2022 18:45
@jwoertink jwoertink added the BREAKING CHANGE This will cause a breaking change label May 1, 2022
Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if for the precompiled tasks, we just remove all of them. Then we somehow create a single command that builds all of them at development time. This would allow you to precompile some additional tasks like db.console since we'd know your final config at that time 🤔

In any case, I'll merge this now to get started on cleaning things up. I think this is going to help solve the param issue too, but that'll come a bit later.

@jwoertink jwoertink merged commit 3a26731 into luckyframework:main May 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE This will cause a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants