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 options to Command::Base::File #532

Merged
merged 2 commits into from
Mar 20, 2016

Conversation

hico-horiuchi
Copy link
Contributor

This pull request is reference to itamae-kitchen/itamae#191.

  • Add -p option to copy to preserve attributes.
  • Add -R option to copy to copy a directory.
  • Add -R option to change_mode, change_owner and change_group.

@hico-horiuchi hico-horiuchi force-pushed the add_options_to_command_base_file branch 2 times, most recently from 934a290 to 8fdfd65 Compare March 18, 2016 08:19
@mizzy
Copy link
Owner

mizzy commented Mar 18, 2016

Thanks!

-p option is okay. This should be default behavior.

But I don't think -R option should be default and it breaks the backward compatibility.

So how about the code like this ?

def change_mode(file, mode, recursive=false)
  option = "-R" if recursive
  "chmod #{option} #{mode} #{escape(file)}"
end

@hico-horiuchi
Copy link
Contributor Author

@mizzy That's great! I'll fix soon.

@hico-horiuchi hico-horiuchi force-pushed the add_options_to_command_base_file branch from 8fdfd65 to b7e5547 Compare March 20, 2016 12:21
@hico-horiuchi hico-horiuchi force-pushed the add_options_to_command_base_file branch from b7e5547 to 64702a2 Compare March 20, 2016 12:27
@hico-horiuchi
Copy link
Contributor Author

@mizzy I fixed arguments. Would you confirm them?

@mizzy
Copy link
Owner

mizzy commented Mar 20, 2016

LGTM! Thanks!

mizzy added a commit that referenced this pull request Mar 20, 2016
…e_file

Add options to Command::Base::File
@mizzy mizzy merged commit 39a82a3 into mizzy:master Mar 20, 2016
@mizzy
Copy link
Owner

mizzy commented Mar 20, 2016

Released as v2.54.0.

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