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

Processes ERB in yaml config files for page layouts, cells, and elements #760

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@phaedryx
Contributor

phaedryx commented Apr 7, 2015

  • In Rails you can put ERB in the config files (e.g. database.yml) and it will be parsed before it is loaded. This pull request makes alchemy consistent with Rails behavior.
  • It will allow parts of a page to be generated programatically (which I would like to try)
  • It will allow me to include yaml files in other yaml files (which will help my team structure their code better)
@TeatroIO

This comment has been minimized.

Show comment
Hide comment
@TeatroIO

TeatroIO Apr 7, 2015

I've prepared a stage to preview changes. Open stage or view logs.

TeatroIO commented Apr 7, 2015

I've prepared a stage to preview changes. Open stage or view logs.

@tvdeyen

This comment has been minimized.

Show comment
Hide comment
@tvdeyen

tvdeyen Apr 7, 2015

Member

Ok, thanks. @mamhoff @robinboening what do you think about this feature? For me it seems ok, since we cache the files anyway and they are not faced to the admin user and therefore can't break anything.

Member

tvdeyen commented Apr 7, 2015

Ok, thanks. @mamhoff @robinboening what do you think about this feature? For me it seems ok, since we cache the files anyway and they are not faced to the admin user and therefore can't break anything.

@mamhoff

This comment has been minimized.

Show comment
Hide comment
@mamhoff

mamhoff Apr 7, 2015

Contributor

I like the feature! 👍

Contributor

mamhoff commented Apr 7, 2015

I like the feature! 👍

@robinboening

This comment has been minimized.

Show comment
Hide comment
@robinboening

robinboening Apr 7, 2015

Contributor

👍 me too! I don't see any drawbacks. Thanks for contributing!

The only thing I noticed is there could be tests for the erb parsing.

Contributor

robinboening commented Apr 7, 2015

👍 me too! I don't see any drawbacks. Thanks for contributing!

The only thing I noticed is there could be tests for the erb parsing.

@tvdeyen

This comment has been minimized.

Show comment
Hide comment
@tvdeyen

tvdeyen Apr 7, 2015

Member

Yes, tests would be great. @phaedryx could you please add some? Thanks.

Member

tvdeyen commented Apr 7, 2015

Yes, tests would be great. @phaedryx could you please add some? Thanks.

@tvdeyen tvdeyen added this to the 3.2 milestone Apr 7, 2015

@phaedryx

This comment has been minimized.

Show comment
Hide comment
@phaedryx

phaedryx Apr 8, 2015

Contributor

As these are all private methods, I wasn't sure how you wanted them tested.

Contributor

phaedryx commented Apr 8, 2015

As these are all private methods, I wasn't sure how you wanted them tested.

@tvdeyen

This comment has been minimized.

Show comment
Hide comment
@tvdeyen

tvdeyen Apr 8, 2015

Member

You could test the method that calls the private method (I.e. Alchemy::PageLayout.all

Member

tvdeyen commented Apr 8, 2015

You could test the method that calls the private method (I.e. Alchemy::PageLayout.all

@phaedryx

This comment has been minimized.

Show comment
Hide comment
@phaedryx

phaedryx Apr 8, 2015

Contributor

Is this what you're looking for?

Also, should I change 'collect' to 'map'? I was just following the style of the other tests.

Contributor

phaedryx commented Apr 8, 2015

Is this what you're looking for?

Also, should I change 'collect' to 'map'? I was just following the style of the other tests.

@phaedryx

This comment has been minimized.

Show comment
Hide comment
@phaedryx

phaedryx Apr 8, 2015

Contributor

(well, I might as well make hound happy)

Contributor

phaedryx commented Apr 8, 2015

(well, I might as well make hound happy)

@tvdeyen

This comment has been minimized.

Show comment
Hide comment
@tvdeyen

tvdeyen Apr 8, 2015

Member

Look perfect. Thanks

Member

tvdeyen commented Apr 8, 2015

Look perfect. Thanks

@tvdeyen

This comment has been minimized.

Show comment
Hide comment
@tvdeyen

tvdeyen Apr 8, 2015

Member

merged via 8c265ea

Member

tvdeyen commented Apr 8, 2015

merged via 8c265ea

@tvdeyen tvdeyen closed this Apr 8, 2015

@tvdeyen

This comment has been minimized.

Show comment
Hide comment
@tvdeyen

tvdeyen Apr 8, 2015

Member

Thanks \o/

Member

tvdeyen commented Apr 8, 2015

Thanks \o/

@phaedryx phaedryx deleted the phaedryx:erb-in-yaml branch Apr 21, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment