Added --confirm option to skip confirmation prompts #293

Merged
merged 4 commits into from Sep 17, 2012

Conversation

Projects
None yet
4 participants
Contributor

fb55 commented Aug 13, 2012

No description provided.

This pull request passes (merged a87e4f2 into c12c0ec).

Contributor

mmalecki commented Aug 13, 2012

I was wondering if we could achieve that for every yes/no prompt we have in jitsu. Might include modifying prompt module to support overrides. @fb55 do you want to poke at it?

@fb55 fb55 renamed --force to --confirm, use jitsu.prompt.confirm for checks
while --confirm isn't inline with other command line tools, it allows
the usage of prompt-internal overwrites
c996f75
Contributor

fb55 commented Aug 14, 2012

@mmalecki It did already ;)

This pull request fails (merged c996f75 into c12c0ec).

This pull request passes (merged c0ba011 into c12c0ec).

Contributor

mmalecki commented Aug 15, 2012

Looks like I still have to confirm overriding the package, even if I launch jitsu with --confirm:

[maciej@pc08 test-app]$ jitsu deploy --confirm
info:    Welcome to Nodejitsu nodejitsu
info:    It worked if it ends with Nodejitsu ok
info:    Executing command deploy
info:    Analyzing your application dependencies in app.js
warn:    Local version appears to be old
warn:    Your package.json version will be incremented for you automatically
warn:    About to write /Users/maciej/test-app/package.json
data:    
data:    {
data:        scripts: { start: 'app.js' },
data:        version: '0.3.5-35',
data:        subdomain: 'test-app-21421',
data:        name: 'test-app',
data:        engines: { node: '0.8.x' }
data:    }
data:    
prompt: Is this ok?: (yes) 
Contributor

fb55 commented Aug 16, 2012

feedy:example-app felix$ ../../../bin/jitsu package create
info:    Welcome to Nodejitsu 
info:    It worked if it ends with Nodejitsu ok
info:    Executing command package create
info:    Analyzing your application dependencies in server.js
info:    Found new dependencies. They will be added automatically
data:    { foo: '*' }
warn:    About to write /jitsu/test/fixtures/example-app/package.json
warn:    Using '*' as version for dependencies may eventually cause issues.
warn:    Please use specific versions for dependencies to avoid future problems.
data:    
data:    {
data:        scripts: { start: 'server.js' },
data:        name: 'example-app',
data:        version: '0.0.0-2',
data:        engines: { node: '0.6.x' },
data:        dependencies: { foo: '*' },
data:        subdomain: 'example-app'
data:    }
data:    
prompt: Is this ok?: ^C
feedy:example-app felix$ ../../../bin/jitsu package create --confirm
info:    Welcome to Nodejitsu 
info:    It worked if it ends with Nodejitsu ok
info:    Executing command package create
info:    Analyzing your application dependencies in server.js
info:    Found new dependencies. They will be added automatically
data:    { foo: '*' }
warn:    About to write /jitsu/test/fixtures/example-app/package.json
warn:    Using '*' as version for dependencies may eventually cause issues.
warn:    Please use specific versions for dependencies to avoid future problems.
data:    
data:    {
data:        scripts: { start: 'server.js' },
data:        name: 'example-app',
data:        version: '0.0.0-2',
data:        engines: { node: '0.6.x' },
data:        dependencies: { foo: '*' },
data:        subdomain: 'example-app'
data:    }
data:    
info:    Creating tarball for example-app
info:    Tarball for example-app at /Users/felix/.jitsu/tmp/-example-app-0.0.0-2.tgz
info:    Nodejitsu ok

Are you sure you're running the right version?

@ghost

ghost commented Aug 31, 2012

Where has it gone? The whole source code pieces with confirm and force are missing in the actual master branch. Could it be that a merged failed or got overridden?

Contributor

fb55 commented Sep 1, 2012

@wibuni This pull request just wasn't merged yet :)

Contributor

jfhbrook commented Sep 17, 2012

This looks ready to merge to me, but it looks like this could use some testing.

@jfhbrook jfhbrook merged commit 8a25ed3 into nodejitsu:master Sep 17, 2012

Contributor

jfhbrook commented Sep 17, 2012

Cool, got this merged in. Thanks a bunch @fb55 !

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