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

Remove duplicate code on recipes #16

Open
faustinoaq opened this issue May 21, 2018 · 7 comments
Open

Remove duplicate code on recipes #16

faustinoaq opened this issue May 21, 2018 · 7 comments
Labels
kind:enhancement New feature or request status:help-wanted Extra attention is needed status:on-hold on hold - do not merge

Comments

@faustinoaq
Copy link
Contributor

faustinoaq commented May 21, 2018

Hi 😅

Currently --recipe flag is working very nice on master (not released yet), also the recipes are being automatically tested and deployed 🎉 https://github.com/amberframework/recipes/releases

Although, this repo has some duplicated code across recipes, we have some options to fix this:

  1. Use symlinks to reuse existent code/directories/files, before deploy the symlinks should be replaced by the real folder/file, once deployed and zipped the recipe should not contain symlinks. 😄

  2. Use multiple liquid layouts to reuse templates. We would need a common layout directory, so every recipe can include it and reuse the code. 🎉

  3. Use one recipe per repository this won't remove the duplicated code, but just move the code to a new repo for each recipe. This would work by using shards install, although the maintainability is not easy, because we will require to test, maintain, deploy and review issue/PRs for all recipe repositories.. ❤️

  4. hierarchical recipes where a recipe depends on another base recipe and is effectively a delta from the base recipe by @damianham 👍

/cc @drujensen @damianham @amberframework/contributors @amberframework/core-team

@faustinoaq faustinoaq added kind:enhancement New feature or request status:help-wanted Extra attention is needed status:on-hold on hold - do not merge labels May 21, 2018
@faustinoaq
Copy link
Contributor Author

faustinoaq commented May 21, 2018

Please vote using

  1. 😄 (option 1: symlinks)
  2. 🎉 (option 2: luiquid layouts)
  3. ❤️ (option 3: multiples repos)
  4. 👍 (option 4: hierarchical recipes)

@damianham
Copy link
Contributor

damianham commented May 21, 2018

I think my preferred option to remove duplicate code in recipes is to have hierarchical recipes where a recipe depends on another base recipe and is effectively a delta from the base recipe. I am working on some options to refactor recipes. In particular I am looking to source a recipe directly from a github repo as per the comment by @drujensen i.e.

amber new myapp -r drujensen/basic_granite

which would source the recipe from https://github.com/drujensen/basic_granite repository. However I am not thinking about going the whole 9 yards with a shards based installation as that will imply copying a lot of code from the shards program into the amber CLI rather than just 1 or 2 files. I am more inclined to a simple a solution as possible that works that just clones and extracts the repo. Once I have this feature working I can experiment with hierarchical recipes more easily. With the implementation of hierarchical recipes I envisage the above command would become;

amber new myapp -r drujensen/granite

which would extend the base recipe at e.g. amberframework/base and add the granite ORM.

@faustinoaq
Copy link
Contributor Author

hierarchical recipes

@damianham Oh yeah! I added your option, and I think is a very nice idea 👍

@drujensen
Copy link
Member

@damianham Thanks for looking into this.

@drujensen
Copy link
Member

Would be nice shards install drujensen/basic_granite existed where it would add this to your shards.yml and install it. https://github.com/crystal-lang/shards/blob/master/src/commands/install.cr

But there is still a chicken/egg thing since the shards.yml won't exist until the project templates are ran. It would require a bootstrap step which is probably what you are already doing.

I still think it would be nice to be able to update the recipes by just updating the shard.yml.

@faustinoaq
Copy link
Contributor Author

@drujensen I think shards install doesn't support that yet 😅

See: install cli

See: crystal-lang/shards#144

@damianham
Copy link
Contributor

@drujensen yes there is a chicken/egg situation and I am trying to keep it as simple as possible. I will see if I can find a method to specify the recipe in the app/shard.yml and also bootstrap the app from the recipe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:enhancement New feature or request status:help-wanted Extra attention is needed status:on-hold on hold - do not merge
Projects
None yet
Development

No branches or pull requests

3 participants