Added ability to add args to callbacks after and before.

[miry]

Commet from @michamilz: http://www.jetthoughts.com/tech/2013/09/26/capistrano-3-passing-parameters-to-your-task.html#comment-1233054544

Added ability to provide args to callbacks. Example we have task with args:

desc 'Example of task with args'
task :uname, :opt do |task, args|
  on roles(:app) do
    execute :uname, args[:opt]
  end
end

To provide args for callbacks in current version:

before :started, "uname_a" do
  Rake::Task[:uname].invoke('-a')
end

Added a new implementation:

before :started, :uname, '-a'

[miry]

@seenmyfate can you review. add help me to write test for callbacks after and before

[leehambley]

@miry, @seenmyfate has withdrawn from the core team to focus on some other things. I'll review this with you sometime. Regarding testing before and after hooks, you might look at https://github.com/guillermo/rake-hooks where the original implementation was inspired by.

[miry]

@leehambley thanks

[leehambley]

Can you please add a corresponding section to the docs?

[rosenfeld]

Looking at the rake's source, it seems invoke will call all prerequisites and finally the @action which come from the blocks. The first action in the array is the task definition itself. So before is acting like after after this change.

[rosenfeld]

if you look at both implementations for before and after you'll notice that they both implement the same thing after this change... I'd suggest to revert this and release a 3.2.1 and then we could think about adding some specs and discussing other possible options to achieve the requested feature...

[leehambley]

Let's just try and fix this for 3.2.1, reverting makes no sense, most
people have bundler or something. I'm away from my computer for a few
hours, perhaps someone could prepare a PR and I can ship 3.2.1 this evening
when I get back to my computer.

Sent from my Nexus 4.
On 15 Apr 2014 16:22, "Rodrigo Rosenfeld Rosas" notifications@github.com
wrote:

if you look at both implementations for before and after you'll notice
that they both implement the same thing after this change... I'd suggest to
revert this and release a 3.2.1 and then we could think about adding some
specs and discussing other possible options to achieve the requested
feature...

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/928#issuecomment-40486986
.

[rosenfeld]

Sorry, I don't understand why reverting this commit doesn't make sense because people have bundler... This commit was merged just a few hours ago and it breaks before hooks. I don't think releasing a broken 3.2.0 makes sense and releasing 3.2.1 with the fixed before hook makes sense to me and would allow some time for people to test this new feature with another working implementation. I can't think of any trivial way to achieve this feature request the proper way. Most probably a new task should be created on-the fly and be used as a prerequisite for this to work.

[rosenfeld]

Also, this feature is not described in the Changelog anyway, so people are probably not using it yet, specially because 3.2.0 was just released.

[leehambley]

@miry if you don't fix this within the next couple of hours, I have no
choice but to revert it, sorry, I can't see a way to make calling
prerequisites with with args work cleanly. Also in light of the links to SO
posted, I don't see how it can be done. Admittedly I'm reading and working
on my phone whilst sitting at the Govt vehicle registration offices, not an
ideal situation.

Sent from my Nexus 4.

Also, this feature is not described in the Changelog anyway, so people are
probably not using it yet, specially because 3.2.0 was just released.

—
Reply to this email directly or view it on
GitHubhttps://github.com//pull/928#issuecomment-40489610
.

[rosenfeld]

I believe PR #1005 implements this feature the right way and is a good candidate for 3.2.1.

[miry]

#1005 is good

[leehambley]

Alright, but I'm waiting on @juanibiapina to acknowledge himself according to the CONTRIB guidelines.

[scottsuch]

I'm actually looking to take advantage of passing args to a task and have been struggling a bit. From the Changelog and this conversation and #1005 that it wasn't released. Is this still something that is on the table?

[leehambley]

Almost certainly not, it was confusing and the two authors of various patches couldn't decide on the semantics. I would sincerely recommend moving complex stuff like this into a class, and passing the execution context (binding, in any on() block) to this class, along with the relevant arguments.

[rosenfeld]

@scottsuch, would you mind explaining to us what is your concrete use case? I remember none of the authors were able to explain exactly how this feature could help them. It would certainly help to understand how people would be using such a feature.

Also, we could provide you other alternatives to your use case and we could discuss how they would differ from the solution proposed in this issue.

[scottsuch]

@rosenfeld, I have a very simple gem that I'm attempting to refactor. Here is a gist with some comments on what I'm attempting, any feedback is welcome.

[rosenfeld]

Hi @scottsuch, sorry but we have deployed a new release last night and I'm still getting some bug reports and have been working on them. That gist is a bit long for me to have a glance over it. It would help a lot if you could explain how you'd use the after hooks with the params if this PR had been merged to Capistrano. Could you please explain how it would help you?

[scottsuch]

Currently I have two tasks that perform identical commands, but one sets a variable to "deploy" the other to "rollback".
after 'deploy:updated', 'deploy:post_graphite_deploy'
after 'deploy:reverted', 'deploy:post_graphite_rollback'

Right now the only way for me to tell whether we are rolling back or deploying is to call a task with after and set the action variable in that task.

If I could pass an argument, I could then use a single task and just pass an "action" argument as a variable and reduce code duplication

I'd use the argument like so:
after 'deploy:updated', 'deploy:post_graphite_event', "deploy"
after 'deploy:reverted', 'deploy:post_graphite_event', "rollback"

Does this make sense?

[rosenfeld]

I commented in the gist. Please take a look. Also, notice that if I understand correctly, even this PR implementation (or the proposed fixes) wouldn't allow your post_graphite_event to be called multiple times in case both deploy:updated and deploy:reverted can be invoked in a single cap run.

[scottsuch]

I'm just responding here to mention that @rosenfeld's comments on my gist were exactly what I was looking for. Thanks again for the help folks.