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

[CLI] change the default recipe source URL #764

Merged
merged 12 commits into from
May 27, 2018

Conversation

damianham
Copy link
Contributor

@damianham damianham commented Apr 19, 2018

Issue: #750
RE: amberframework/recipes#3

Change the default recipe source URL to add the dist folder and support
URLs as a recipe name.

  • changes the default recipe source URL
  • adds support for a URL as a recipe name

Description of the Change

Alternate Designs

Benefits

Possible Drawbacks

Issue: amberframework#750
RE: amberframework/recipes#2

Change the default recipe source URL to add the dist folder and support
URLs as a recipe name.

- changes the default recipe source URL
- adds support for a URL as a recipe name
damianham and others added 2 commits April 23, 2018 19:02
remove spec testing recipe in cache in parent folder
@damianham
Copy link
Contributor Author

This is ready to be merged - we have changed the recipes repository to match this PR

faustinoaq
faustinoaq previously approved these changes Apr 30, 2018
Copy link
Contributor

@faustinoaq faustinoaq left a comment

Choose a reason for hiding this comment

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

🎉

@faustinoaq
Copy link
Contributor

Requires #783 to fix Travis specs

BTW, I already working on .zip deployment for amber recipes, and writing some comments about it.

I know @drujensen have some concerns, although I still think recipes is a great Idea,

@drujensen recipes are not shards, they are just pre-created amber projects 😅

The main issue is because we (some of us, including me 😉 ) want to store all blessed recipes in only one repository. This have some pitfalls, although I think I already managed to solved them, just let me publish my ideas (still writing them 😅 )

@damianham damianham mentioned this pull request May 13, 2018
@faustinoaq faustinoaq dismissed their stale review May 13, 2018 21:14

Travis is failing

Copy link
Contributor

@faustinoaq faustinoaq left a comment

Choose a reason for hiding this comment

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

We should fix Travis spec 👇

Error opening file '/home/travis/build/amberframework/amber/tmp/test_app/shard.yml' with mode 'r': No such file or directory (Errno)
  from /usr/share/crystal/src/file.cr:33:5 in 'initialize'
  from /usr/share/crystal/src/file.cr:25:3 in 'new'
  from /usr/share/crystal/src/file.cr:427:5 in 'read'
  from /usr/share/crystal/src/file.cr:441:3 in 'read'
  from spec/support/helpers/cli_helper.cr:80:13 in 'prepare_yaml'
  from spec/support/helpers/cli_helper.cr:100:5 in 'scaffold_app'
  from /usr/share/crystal/src/spec/expectations.cr:0:7 in '~procProc(Nil)'
  from /usr/share/crystal/src/spec/context.cr:255:3 in 'describe'
  from /usr/share/crystal/src/spec/methods.cr:16:5 in 'describe'
  from /usr/share/crystal/src/spec/methods.cr:25:5 in 'context'
  from /usr/share/crystal/src/spec/methods.cr:0:5 in '~procProc(Nil)'
  from /usr/share/crystal/src/spec/context.cr:255:3 in 'describe'
  from /usr/share/crystal/src/spec/methods.cr:16:5 in 'describe'
  from spec/amber/cli/commands/init_spec.cr:7:3 in '__crystal_main'
  from /usr/share/crystal/src/crystal/main.cr:11:3 in '_crystal_main'
  from /usr/share/crystal/src/crystal/main.cr:112:5 in 'main_user_code'
  from /usr/share/crystal/src/crystal/main.cr:101:7 in 'main'
  from /usr/share/crystal/src/crystal/main.cr:135:3 in 'main'
  from __libc_start_main
  from ???
  from ???

@damianham Any idea why is this failing?

Please see: https://travis-ci.org/amberframework/amber/builds/378226262#L696 😅

@faustinoaq
Copy link
Contributor

Hi @damianham I think we should update the URL to:

"https://github.com/amberframework/recipes/releases/download/dist/"

WDYT?

Ref: https://github.com/amberframework/recipes/releases

@drujensen
Copy link
Member

@faustinoaq @damianham what is the status on this?

faustinoaq
faustinoaq approved these changes May 27, 2018
@faustinoaq
Copy link
Contributor

@drujensen This ready and is required by recipes, I'm already working on removing duplicated code from recipes, see: amberframework/recipes#16 I think we should try current recipe implementation by @damianham , then, after some iterations we can try to move it to shards or something better 👍

@faustinoaq faustinoaq requested review from a team May 27, 2018 16:38
@drujensen drujensen merged commit fcb7e4b into amberframework:master May 27, 2018
@faustinoaq faustinoaq added this to the Version 0.8.0 milestone Jun 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants