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

Fix a bug that displays inappropriate diff in file deletion #200

Merged
merged 2 commits into from
Mar 6, 2016

Conversation

takus
Copy link
Contributor

@takus takus commented Feb 17, 2016

It seems that template resource's :delete action doesn't work as what I suppose. It always displays contents of its source. I think itamae should suppress diff in :delete action like below.

before

itamae local bootstrap.rb
 INFO : Starting Itamae...
 INFO : Recipe: /opt/itamae/bootstrap.rb
 INFO :   Recipe: /opt/itamae/cookbooks/java/tomcat.rb
 INFO :     template[/usr/share/tomcat7/newrelic/newrelic.yml] exist will change from 'true' to 'false'
 INFO :     diff:
 INFO :     --- /usr/share/tomcat7/newrelic/newrelic.yml        2016-02-17 11:12:18.541089617 +0000
 INFO :     +++ /tmp/itamae_tmp/1455707545.4180827      2016-02-17 11:12:25.413097913 +0000
 INFO :     @@ -0,0 +1,277 @@
 INFO :     +# This file configures the New Relic Agent.  New Relic monitors
 INFO :     +# Java applications with deep visibility and low overhead.  For more details and additional
 INFO :     +# configuration options visit https://docs.newrelic.com/docs/java/java-agent-configuration.
....

after

itamae local bootstrap.rb
 INFO : Starting Itamae...
 INFO : Recipe: /opt/itamae/bootstrap.rb
 INFO :   Recipe: /opt/itamae/cookbooks/java/tomcat.rb
 INFO :     template[/usr/share/tomcat7/newrelic/newrelic.yml] exist will change from 'true' to 'false'
...

Although this code passes tests, I'm not sure this is a good way to suppress inappropriate diff. Plz give me feedback if you guys have good ideas.

@ryotarai
Copy link
Member

Thank you for your pull request.

It always displays contents of its source. I think itamae should suppress diff in :delete action like below

I agree that.

I'm not sure this is a good way to suppress inappropriate diff. Plz give me feedback if you guys have good ideas.

It is better to modify show_differences method in File class, because delete action of template resource is actually delete action of file resource.
How about fixing

like the following?

-        if @temppath
+        if @temppath && @current_action != :delete
           compare_file
         end

@takus
Copy link
Contributor Author

takus commented Feb 17, 2016

Thanks for the comment. Your suggestion seems better. I've fixed it.

@ryotarai
Copy link
Member

ryotarai commented Mar 6, 2016

Sorry for late response. I'm about to merge this.
Thank you for your contribution.

ryotarai added a commit that referenced this pull request Mar 6, 2016
Fix a bug that displays inappropriate diff in file deletion
@ryotarai ryotarai merged commit b520cb8 into itamae-kitchen:master Mar 6, 2016
@takus
Copy link
Contributor Author

takus commented Mar 6, 2016

Thanks 🍣

@takus takus deleted the bugfix branch March 6, 2016 14:12
@ryotarai
Copy link
Member

ryotarai commented Mar 6, 2016

released v1.9.4

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