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

add cli new #233

Merged
merged 1 commit into from
Feb 4, 2016
Merged

add cli new #233

merged 1 commit into from
Feb 4, 2016

Conversation

mpapis
Copy link
Collaborator

@mpapis mpapis commented Jan 31, 2016

Usage: pre-commit new plugin-name 'Author Name' author@email 'description of the plugin'

@jish I hope you like it :) is there anything I could do better (except adding tests)?

=end

if
RUBY_VERSION == "2.0.0" # check Gemfile
Copy link

Choose a reason for hiding this comment

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

Did you mean >= '2.0.0' ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

coverage is calculated only on this single ruby, needed gems are restricted only to this ruby https://github.com/jish/pre-commit/pull/233/files#diff-528248a33c4d64f323b7c6b622a98c54R13

on one hand it was needed for older rubies (I think redcarpet was not installing)
but it also helps to save workers time / test time for what it counts


module PreCommit
class Template
TEMPLATE_DIR = File.expand_path("../../../templates/gem", __FILE__)
Copy link
Owner

Choose a reason for hiding this comment

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

We might want to call this templates/plugin. Not a big deal though, this is fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking later we could extend the new command, then we could have a way to generate more things like just plugin in existing gem, possibly a plugin template could be called from gem template

@jish
Copy link
Owner

jish commented Feb 3, 2016

looks good if we can get tests passing.

@mpapis mpapis force-pushed the feature/cli_new_plugin branch 2 times, most recently from 1ad5caf to 81a659d Compare February 3, 2016 21:45
@mpapis
Copy link
Collaborator Author

mpapis commented Feb 3, 2016

applied suggestions and tests are passing

attr_reader :name, :author, :email, :description, :gem_name, :copyright

def initialize(*args)
@name, @author, @email, @description = args
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm really unhappy with this, it's good for start, but later on it would make sense to allow saving @author, @email in configuration and only if missing require them maybe as last param in form 'Name <email@...>'

jish added a commit that referenced this pull request Feb 4, 2016
@jish jish merged commit 43687ee into master Feb 4, 2016
@jish jish deleted the feature/cli_new_plugin branch February 4, 2016 16:31
@mpapis
Copy link
Collaborator Author

mpapis commented Feb 11, 2016

here is the extracted tool: https://github.com/mpapis/tree_renderer - @jish should I open new PR making use of it or you are fine with current implementation?

@jish
Copy link
Owner

jish commented Feb 16, 2016

I think the current implementation is ok.

Good job extracting the functionality :) In this case I think it is small enough that it does not warrant adding a new gem dependency.

Josh

On Feb 11, 2016, at 01:26, Michal Papis notifications@github.com wrote:

here is the extracted tool: https://github.com/mpapis/tree_renderer - @jish should I open new PR making use of it or you are fine with current implementation?


Reply to this email directly or view it on GitHub.

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