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

Print '(dry-run)' first in dry-run mode #252

Merged
merged 1 commit into from
Oct 15, 2018

Conversation

unasuke
Copy link
Member

@unasuke unasuke commented Dec 6, 2017

In dry-run mode, add "(dry-run)" in "Starting Itamae" message.

$ bundle exec itamae ssh --user foo --host example.com role.rb --dry-run
 INFO : Starting Itamae... (dry-run)
 ....

motivation

I use codenize-tools often. It's print "dry-run" as a first in dry-run mode.

e.g as roadworker,

$ bundle exec roadwork --apply --dry-run
Apply `Routefile` to Route53 (dry-run)
No change

https://github.com/codenize-tools/roadworker

The behavior is human-friendly I think. So I want to bring its behavior in itamae.

concern

Itamae::Runner.run is the class method. In instanced Itamae::Runner has @option attribute and dry_run? method.
I want to use it but it's not instanced in the location of the first log message. So reading options variable as direct.

Itamae.logger.info "Starting Itamae... #{options[:dry_run] ? '(dry-run)' : ''}"

@sue445
Copy link
Member

sue445 commented Oct 14, 2018

@unasuke Sorry for late reply.

I think this is LGTM, but CI is broken at your branch. Please rebase (or merge ) latest master.

@sue445 sue445 self-requested a review October 15, 2018 01:10
@unasuke unasuke self-assigned this Oct 15, 2018
@sue445 sue445 merged commit a5403ae into itamae-kitchen:master Oct 15, 2018
@unasuke unasuke deleted the dry_run branch October 15, 2018 04:10
@sue445
Copy link
Member

sue445 commented Oct 16, 2018

@unasuke v1.9.12 is released 💎

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.

2 participants