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

[WIP] Extract CLI generator #931

Closed
wants to merge 31 commits into from
Closed

[WIP] Extract CLI generator #931

wants to merge 31 commits into from

Conversation

cllns
Copy link
Member

@cllns cllns commented May 11, 2018

We'd like to extract to file generation functionality from this repo to Hanami::CLI.

A few reasons:

  1. So other users of Hanami::CLI can benefit from our work. They won't need to write their own file manipulation and output code.
  2. We want to colorize the CLI file generation, and handle that at the Hanami::CLI level, so all our specs for this repo can be plaintext without being muddied by a bunch of shell-code for colorizing (since we can disable colorizing for specs, and delegate the testing of that behavior to Hanami::CLI's test suite).
  3. We can add a feature for prompting users when they'd potentially overwrite a file that already exists. We could've done this here, but it'll be simpler as its own feature in Hanami::CLI.

cc @AlfonsoUceda @hanami/core

Some clean-up too:

  • I changed some variables names to path from destination when there's only one page. It makes sense to have destination and source when they are two paths, but, for example, destination doesn't make sense for something we want to delete.

  • The CLI said a route was inserted, when really it was appended. Either one works but it's simpler for the cade to just let it say the route was appended to the routes file.

cllns added 11 commits May 10, 2018 21:25
Also renamed from 'destination' to 'path', since a destination doesn't
really make sense for something that's being deleted.
And rename some variables from 'destination' to 'path', since it's more
accurate
Also fix the behavior for routes, where it'd say it 'inserted' the route
when really it appended it. Either would be fine, but this is more
accurate and simpler to implement.
@cllns cllns added this to the v1.3.0 milestone May 11, 2018
@cllns cllns changed the base branch from master to develop May 11, 2018 05:37
@AlfonsoUceda
Copy link
Contributor

AlfonsoUceda commented Jun 29, 2018

very good job @cllns it makes sense to extract this to CLI, for me it is ok :)

Can you rebase this branch from develop? and if you can finish something you left, please finish it;)

@cllns
Copy link
Member Author

cllns commented Jul 9, 2018

Talking with @jodosha a little while ago, we decided this should be extracted more. Specifically, reading the templates should be extracted into hanami-cli.

So instead of:

source = templates.find("README.md.erb")
destination = project.readme(context)
generator.create(source, destination, context)

The API would be:

destination = project.readme(context)
generator.create("README.md.erb", destination, context)

Does that make sense? It's quite a bit more work on top of what's already in this PR.

@AlfonsoUceda AlfonsoUceda removed this from the v1.3.0 milestone Jul 19, 2018
@AlfonsoUceda
Copy link
Contributor

Removing milestone 1.3.0 because it isn't completed and is not a priority

@jodosha
Copy link
Member

jodosha commented Oct 17, 2018

Closing as stale PR. @cllns Feel free to resume the work.

@jodosha jodosha closed this Oct 17, 2018
@jodosha jodosha deleted the extract-cli-generator branch October 17, 2018 09:26
@cllns cllns reopened this Jan 10, 2019
@cllns cllns added enhancement and removed wontfix labels Jan 10, 2019
@jodosha
Copy link
Member

jodosha commented Sep 18, 2019

Closing as stale initiative. @cllns Feel free to reopen, in case.

@jodosha jodosha closed this Sep 18, 2019
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