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

CLI Framework #12

Merged
merged 4 commits into from Jul 6, 2015
Merged

CLI Framework #12

merged 4 commits into from Jul 6, 2015

Conversation

nkbt
Copy link
Contributor

@nkbt nkbt commented Jul 4, 2015

I reckon it is a part of #7

This is how it works:

cli-framework

So now we have 3 "binaries":

  1. cloverfield
  2. cf - convenient alias for cloverfield
  3. cf-package - stub for a package scaffold (will be implemented according to Use prod module boilerplate #10)

cf has only one command available for now: build with a single option <scaffold>. See code comments for details.

cf build package is equivalent to cf-package and starts step-by-step "wizard".

Regarding libs choice. I checked different commands, args and prompt libs and the most friendly seems to be nomnom for commands and options and prompt for, hm.., prompt. I used other libs before (like commander.js) and found them quite syntax-heavy and not fun.

If we are ok with tooling, I can actually implement scaffold/package #10 now. Should be pretty straightforward.

PS: CLI colors and messages can be customized

@@ -0,0 +1,35 @@
#! /usr/bin/env ./node_modules/.bin/babel-node
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a nice trick to run CLI-scripts with babel-node

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool!

@nkbt nkbt assigned nkbt and ericelliott and unassigned nkbt Jul 4, 2015
@@ -0,0 +1,32 @@
# Created by .ignore support plugin (hsz.mobi)
Copy link
Contributor

Choose a reason for hiding this comment

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

If you've made general improvements to this file that would be useful to prod-module-boilerplate, could you make sure they're reflected there, too? I haven't looked myself...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is quite common. Created if you make a new Github project and choose Node as default .gitignore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@ericelliott
Copy link
Contributor

I don't see any unit tests. Did I miss something?

@ericelliott
Copy link
Contributor

Over-all, great work. 👏

@nkbt
Copy link
Contributor Author

nkbt commented Jul 4, 2015

I wasn't sure about tests, since they require additional libs pulled into a project. As well as eslint. Was thinking of doing it in a separate PR to keep focus on CLI frameworks itself in this one.

@ericelliott
Copy link
Contributor

Consider testing a requirement for all substantial code commits. Since we don't have the supporting stuff added here yet, feel free to add them with this initial PR. =)

@ericelliott
Copy link
Contributor

And thanks for being a rockstar on this project lately. 🎸 🔥

@nkbt
Copy link
Contributor Author

nkbt commented Jul 5, 2015 via email

"max-statements": [0, 10],
"new-parens": [2],
"new-cap": [0],
"newline-after-var": [2],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Set it to required

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, need that update there too. It actually gets bit annoying to propagate changes. I've got the same issues with my own modules. Not sure what is the best solution. Maybe some "defaults" repo with stuff and couple of scripts like npm install && cp -f ./node_modules/defaults/.eslintrc ./?
What do you reckon?

ericelliott pushed a commit that referenced this pull request Jul 6, 2015
@ericelliott ericelliott merged commit 35afbd8 into cloverfield-tools:master Jul 6, 2015
@nkbt
Copy link
Contributor Author

nkbt commented Jul 6, 2015

Blue-tape does not have CLI support (the one tape has). I've just made a PR that fixes this issue: spion/blue-tape#12

Until it is fixed, I have to add some automation to our tests.

@nkbt nkbt deleted the cli-framework branch July 6, 2015 10:39
@ericelliott
Copy link
Contributor

It does now. =)

@nkbt
Copy link
Contributor Author

nkbt commented Jul 7, 2015

Seen it. Will remove tape dependency later today

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.

None yet

2 participants