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

Render bin #20

Closed
wants to merge 9 commits into from
Closed

Render bin #20

wants to merge 9 commits into from

Conversation

meson10
Copy link

@meson10 meson10 commented Nov 6, 2017

Fixes #5

@meson10
Copy link
Author

meson10 commented Nov 6, 2017

Damn. Something changed in master in past few days because of which this is failing. Will fix and push.

Copy link
Member

@markbates markbates left a comment

Choose a reason for hiding this comment

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

I love this PR, but there are a few things to note before it can be merged.

  1. It breaks the existing functionality of the plush command. Before you could do something like plush foo.plush and it would work, now it raises an error:
Error: unknown command "foo.plush" for "plush"
Run 'plush --help' for usage.
unknown command "foo.plush" for "plush"

That needs to be fixed before this can be merged.

  1. It would be great if the flags also applied to running scripts as well as templates.

  2. The current functionality of the plush command is to run scripts, i.e. "raw" plush. This command adds the ability to run a template. Perhaps they be combined with a flag? Or some other way of distinguishing the two.

  3. Plush using the https://godoc.org/github.com/stretchr/testify/require library for testing, so if you could please keep with the coding style of the project that would be excellent.

Again, overall I love the idea of this, but we need to fix these few things first before we can merge it. Thanks.

@Dexus
Copy link

Dexus commented Jan 8, 2018

Never updated... very nice... tzzz...

@markbates
Copy link
Member

Closing due to lack of progress.

@markbates markbates closed this Mar 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants