Permalink
Browse files

Fix renaming of the deprecated "deploy:symlink" task to "deploy:creat…

…e_symlink"

An earlier commit did not account for the case when the "deploy:symlink" task appears as a "before" or "after" hook.
  • Loading branch information...
1 parent d263896 commit 662270778a36d33f156f3055f539b20386b255b3 @carsomyr carsomyr committed Aug 21, 2012
Showing with 32 additions and 22 deletions.
  1. +22 −16 lib/capistrano/configuration/callbacks.rb
  2. +10 −6 test/configuration/callbacks_test.rb
@@ -19,7 +19,7 @@ def initialize_with_callbacks(*args) #:nodoc:
end
def invoke_task_directly_with_callbacks(task) #:nodoc:
-
+
trigger :before, task
result = invoke_task_directly_without_callbacks(task)
@@ -106,24 +106,30 @@ def on(event, *args, &block)
elsif block
callbacks[event] << ProcCallback.new(block, options)
else
- args.each do |name|
- [:only, :except].each do |key|
- if options[key] == 'deploy:symlink'
- warn "[Deprecation Warning] This API has changed, please hook `deploy:create_symlink` instead of `deploy:symlink`."
- options[key] = 'deploy:create_symlink'
- elsif options[key].is_a?(Array) && options[key].include?('deploy:symlink')
- warn "[Deprecation Warning] This API has changed, please hook `deploy:create_symlink` instead of `deploy:symlink`."
- options[key] = options[key].collect do |task|
- task == 'deploy:symlink' ? 'deploy:create_symlink' : task
- end
- end
- end
-
- callbacks[event] << TaskCallback.new(self, name, options)
- end
+ args = filter_deprecated_tasks(args)
+ options[:only] = filter_deprecated_tasks(options[:only])
+ options[:expect] = filter_deprecated_tasks(options[:expect])
@lest

lest Aug 21, 2012

Looks like a typo. I guess it should be except here.

+
+ callbacks[event].concat(args.map { |name| TaskCallback.new(self, name, options) })
end
end
+ # Filters the given task name or names and attempts to replace deprecated tasks with their equivalents.
+ def filter_deprecated_tasks(names)
+ deprecation_msg = "[Deprecation Warning] This API has changed, please hook `deploy:create_symlink` instead of" \
+ " `deploy:symlink`."
+
+ if names == "deploy:symlink"
+ warn deprecation_msg
+ names = "deploy:create_symlink"
+ elsif names.is_a?(Array)
@lest

lest Aug 21, 2012

It causes unexpected deprecation warnings when called from args = filter_deprecated_tasks(args) because args are always an array.

+ warn deprecation_msg
+ names = names.map { |name| name == "deploy:symlink" ? "deploy:create_symlink" : name }
+ end
+
+ names
+ end
+
# Trigger the named event for the named task. All associated callbacks
# will be fired, in the order they were defined.
def trigger(event, task=nil)
@@ -40,8 +40,10 @@ def test_before_should_delegate_to_on
end
def test_before_should_map_before_deploy_symlink
- @config.before "deploy:symlink", "bing:blang"
- assert_equal ["deploy:create_symlink"], @config.callbacks[:before].last.only
+ @config.before "deploy:symlink", "bing:blang", "deploy:symlink"
+ assert_equal "bing:blang", @config.callbacks[:before][0].source
+ assert_equal "deploy:create_symlink", @config.callbacks[:before][1].source
+ assert_equal ["deploy:create_symlink"], @config.callbacks[:before][1].only
end
def test_before_should_map_before_deploy_symlink_array
@@ -55,13 +57,15 @@ def test_after_should_delegate_to_on
end
def test_after_should_map_before_deploy_symlink
- @config.after "deploy:symlink", "bing:blang"
- assert_equal ["deploy:create_symlink"], @config.callbacks[:after].last.only
+ @config.after "deploy:symlink", "bing:blang", "deploy:symlink"
+ assert_equal "bing:blang", @config.callbacks[:after][0].source
+ assert_equal "deploy:create_symlink", @config.callbacks[:after][1].source
+ assert_equal ["deploy:create_symlink"], @config.callbacks[:after][1].only
end
def test_after_should_map_before_deploy_symlink_array
- @config.after ["deploy:symlink", 'bingo:blast'], "bing:blang"
- assert_equal ["deploy:create_symlink", 'bingo:blast'], @config.callbacks[:after].last.only
+ @config.after ["deploy:symlink", "bingo:blast"], "bing:blang"
+ assert_equal ["deploy:create_symlink", "bingo:blast"], @config.callbacks[:after].last.only
end
def test_on_with_single_reference_should_add_task_callback

4 comments on commit 6622707

lest replied Aug 21, 2012

What if I don't use recipes/deploy and have custom deploy:symlink task? I think it shouldn't be deprecated globally.

Contributor

carsomyr replied Aug 21, 2012

Then bad things will happen. I guess you'll have to think of the deploy:symlink name as still occupied by the deploy recipe.

lest replied Aug 21, 2012

Deploy recipe is not required by default so this deprecation should appear only if it's required.

Contributor

carsomyr replied Aug 21, 2012

I see your point, and it's a good one. Unfortunately, I am merely fixing the regression introduced by someone else, and the change will have to stand. We may remove the deprecation warnings in the future, once people have stopped using the old conventions.

Please sign in to comment.