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 cleanup #808

Merged
merged 13 commits into from Jul 27, 2017

Conversation

Projects
None yet
3 participants
@jodosha
Copy link
Member

jodosha commented Jul 22, 2017

Notable changes

  • Introduces the latest changes from hanami-cli
  • Removes thor as dependency
  • Migrates all the commands as "components": they can declare dependencies. Eg. hanami db migrate depends on the model.configuration component. See Hanami::Cli::Commands::Command for details
  • Adds more control on errors and exit codes (eg. Kernel.exit(1))
  • Let third party developers can add or override commands. (See Third party CLI section below)
  • Adds extended --help banner with examples. (See Command banner section below)
  • Introduces new file manipulation helpers from hanami-utils.
  • Introduces a centralized module to determine where the files should be generate: Eg. where an action for the app web should go? Hanami::Cli::Commands::Project gives you the answer via: .action function

Third party CLI

Let's imagine someone wants to implement a hanami-webpack gem. Because of this PR, they can now do:

module Hanami
  module Webpack
    module CLI
      class Generate < Hanami::Cli::Commands::Command
        desc "Generate Webpack configuration"
        def call(*)
          # ...
        end
      end
    end
  end
end

Hanami::Cli.register "generate webpack", Hanami::Webpack::CLI::Generate

From a bookshelf project that bundles hanami-webpack:

% bundle exec hanami generate
Commands:
  hanami generate action APP ACTION                    # Generate an action for app
  hanami generate app APP                              # Generate an app
  hanami generate mailer MAILER                        # Generate a mailer
  hanami generate migration MIGRATION                  # Generate a migration
  hanami generate model MODEL                          # Generate a model
  hanami generate secret [APP]                         # Generate session secret
  hanami generate webpack                              # Generate Webpack configuration

Command banner

Here's some examples of the new --help banners

% bundle exec hanami new --help
Command:
  hanami new

Usage:
  hanami new PROJECT

Description:
  Generate a new Hanami project

Arguments:
  PROJECT             	# REQUIRED The project name

Options:
  --database=VALUE, -d VALUE      	# Database (mysql/mysql2/postgresql/postgres/sqlite/sqlite3), default: "sqlite"
  --application-name=VALUE        	# App name, default: "web"
  --application-base-url=VALUE    	# App base URL, default: "/"
  --template=VALUE                	# Template engine (erb/haml/slim), default: "erb"
  --test=VALUE                    	# Project testing framework (minitest/rspec), default: "minitest"
  --[no-]hanami-head              	# Use Hanami HEAD (true/false), default: false
  --help, -h                      	# Print this help

Examples:
  hanami new bookshelf                     # Basic usage
  hanami new bookshelf --test=rspec        # Setup RSpec testing framework
  hanami new bookshelf --database=postgres # Setup Postgres database
  hanami new bookshelf --template=slim     # Setup Slim template engine
  hanami new bookshelf --hanami-head       # Use Hanami HEAD
% bundle exec hanami server --help
Command:
  hanami server

Usage:
  hanami server

Description:
  Start Hanami server (only for development)

Options:
  --server=VALUE                  	# Force a server engine (eg, webrick, puma, thin, etc..)
  --host=VALUE                    	# The host address to bind to
  --port=VALUE, -p VALUE          	# The port to run the server on
  --debug=VALUE                   	# Turn on debug output
  --warn=VALUE                    	# Turn on warnings
  --daemonize=VALUE               	# Daemonize the server
  --pid=VALUE                     	# Path to write a pid file after daemonize
  --[no-]code-reloading           	# Code reloading, default: true
  --help, -h                      	# Print this help

Examples:
  hanami server                     # Basic usage (it uses the bundled server engine)
  hanami server --server=webrick    # Force `webrick` server engine
  hanami server --host=0.0.0.0      # Bind to a host
  hanami server --port=2306         # Bind to a port
  hanami server --no-code-reloading # Disable code reloading
% bundle exec hanami db prepare --help
Command:
  hanami db prepare

Usage:
  hanami db prepare

Description:
  Drop, create, and migrate the database (only for development/test)

Options:
  --help, -h                      	# Print this help

Technical notes

Most of the new code don't have API docs yet and need smaller, subsequent refactorings. But I decided to stop at a "good enough" point and revisit it during the 1.1.beta period.

This PR points to non-stable branches of hanami-utils and hanami-cli. It's my duty to finish the work on these branches, but I wanted to give the team the opportunity to review the hanami code in the meantime.

@jodosha jodosha added the enhancement label Jul 22, 2017

@jodosha jodosha added this to the v1.1.0 milestone Jul 22, 2017

@jodosha jodosha self-assigned this Jul 22, 2017

@jodosha jodosha requested review from AlfonsoUceda , oana-sipos and hanami/core Jul 22, 2017

@AlfonsoUceda
Copy link
Member

AlfonsoUceda left a comment

AWESOME @jodosha 🎩

argument :version, desc: "The target version of the migration (see `hanami db version`)"

example [
" # Migrate to the last version",

This comment has been minimized.

@AlfonsoUceda

AlfonsoUceda Jul 23, 2017

Member

do you need to align with the line below?

This comment has been minimized.

@jodosha

def controller_and_action(name)
# FIXME: extract this regexp
name.split(/#|\//)

This comment has been minimized.

@AlfonsoUceda

AlfonsoUceda Jul 23, 2017

Member

Fix this FIXME?

destination = project.environment(context)

line = files.read_matching_line(destination, content)
*, at = line.split(/at\:[[:space:]]*/)

This comment has been minimized.

@AlfonsoUceda

AlfonsoUceda Jul 23, 2017

Member

why not _ instead *? I didn't know you can use that when return an array, how it works?

This comment has been minimized.

@kaikuchn

kaikuchn Jul 24, 2017

Member

On the left of the assignment a destructuring is happening. If you used _ then it would match for only one element of the array, at would then match the second element. * captures as many elements as it can, depending on what comes before and after it. In this case *, at = ... means that at will be the last element of the array and * captures all the rest. Since it doesn't have a name we discard it.
I think the style guide says to use *_ in that case. But I'd argue that here we should just say at = line.split(/at\:[[:space:]]*/).last since that appears to be what we're interested in and it is a more revealing way of stating that.

This comment has been minimized.

@AlfonsoUceda

AlfonsoUceda Jul 24, 2017

Member

@kaikuchn thank you!! IMO if we can replace that with your suggestion, it would improve the read of the code.


def controller_and_action_name(name)
# FIXME: extract this regexp
name.split(/#|\//)

This comment has been minimized.

@AlfonsoUceda

AlfonsoUceda Jul 23, 2017

Member

Fix FIXME?

def init_git
return if git_dir_present?

say(:run, "git init . from \".\"")

This comment has been minimized.

@AlfonsoUceda

AlfonsoUceda Jul 23, 2017

Member

I think we need to add target here, isn't it?

@@ -48,6 +59,45 @@ class AccessToken < Dry::Struct
end
end # irb

xit "returns error when known engine isn't bundled" do

This comment has been minimized.

@AlfonsoUceda

AlfonsoUceda Jul 23, 2017

Member

doesn't it work for now?

@@ -19,5 +19,36 @@
end
end
end

xit 'prints help message' do

This comment has been minimized.

@AlfonsoUceda

AlfonsoUceda Jul 23, 2017

Member

remove the pending?

@@ -0,0 +1,17 @@
RSpec.describe "hanami generate", type: :cli do

This comment has been minimized.

@AlfonsoUceda

AlfonsoUceda Jul 23, 2017

Member

I think you need to add a test using the alias?

@@ -0,0 +1,16 @@
RSpec.describe "hanami destroy", type: :cli do

This comment has been minimized.

@AlfonsoUceda

AlfonsoUceda Jul 23, 2017

Member

I think you need to add a test using the alias, isn't it?

@jodosha

This comment has been minimized.

Copy link
Member

jodosha commented Jul 26, 2017

@AlfonsoUceda I know there are some small things to cleanup, but as stated in the description, I stopped at a "good enough" point, as we need to get this merged to focus on other features for 1.1.beta1.

@jodosha jodosha referenced this pull request Jul 26, 2017

Merged

Introduce `Utils::Files` #223

@AlfonsoUceda

This comment has been minimized.

Copy link
Member

AlfonsoUceda commented Jul 27, 2017

@jodosha it’s good to me, I know there are sometimes we need to stop and split issues. Go ahead ;)

@jodosha jodosha merged commit 72243e1 into develop Jul 27, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jodosha jodosha deleted the cli-cleanup branch Jul 27, 2017

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