include modules instead of inheritance.

[gregory]

include modules instead of inheritance.

[pokonski]

What benefits do you have in mind when using this approach?
On May 17, 2015 7:20 PM, "Grégory Horion" [email protected] wrote:

Hey,

This looks pretty cool. One improvement would be to include modules to
decorate the classes instead of using inheritance.
Here is what i have in mind:

workflows/sample_workflow.rbclass SampleWorkflow

include Gush::Workflow
gush do
run FetchJob1
run FetchJob2, params: {some_flag: true, url: 'http://url.com'}

run PersistJob1, after: FetchJob1
run PersistJob2, after: FetchJob2

run Normalize,
    after: [PersistJob1, PersistJob2],
    before: Index

run Index

endend

class FetchJob1
include Gush::Job
def self.perform(params={})
# do some fetching from remote APIs

params #=> {url: "http://some.com/url"}

endend

what do you think?

—
Reply to this email directly or view it on GitHub
#6.

[gregory]

Using inheritance is a constraint that force me the have classes that are Gush::Workflow or Gush::Job and nothing else, where in practice, it's not always the case.

In the following example, i have my SignupWorkflow that already inherit from a class. Of course, i do not want to make ClassA to inherit from Gush::Workflow cause it is not. In this simple case, how could i make SignupWorkflow a Gush::Workflow?

class ClassA
 # some stuffs
end
class SignupWorkflow < ClassA; end

Same idea for the job, how should i use Gush here and make User::EmailResetPassword a Gush::Job ? :)

class User::EmailResetPassword < SomeClass; end

Adding a specific DSL by including Gush::Workflow might not only allow me to use it with classes that already inherit from another one, but also prevent class method name conflicts.

[pokonski]

Inheriting workflows makes zero sense, because jobs are defined on the instance level. But will change to includes.

[pokonski]

Also the block gush do doesn't really add anything beside pointless complexity.

[mlen]

@gregory the only purpose for *Workflow classes is to provide a DSL for workflows and they're never meant to be instantiated by the users. Look at the DSL that rack uses for configuration. Did you ever need to customize the way how rack apps are configured?

Using custom subclasses for Jobs makes more sense, but I can't see why would it be better than something like this:

class ResetEmailJob < Gush::Job
  def work
    User::EmailResetPassword.new.whatever(params)
  end
end

[gregory]

@pokonski for the gush do i actually meant to avoid using #configure to avoid method name conflict with a class that would have that method already defined...

This doesnt makes sens to you?

class ClassA
  def common_params
  end
end
class Workflow1 < ClassA
  def self.configure
    run FetchJob1, params: common_params
  end
end  
class Workflow2 < ClassA
  def self.configure
    run FetchJob2, params: common_params
  end
end  

@mlen If this is a DSL, why should that be nested in a configure method? The DSL should be enough right?

class ResetEmailJob < Gush::Job
  def work
    User::EmailResetPassword.new.whatever(params)
  end
end

=> Now you are forcing me into your architecture style.
That would mean that in order to have a job, i'd have to create new classes in my codebase just to wrap the call? ...

To be honnest, i'm not even sure why we need to include/inherit from Gush::Job we should just call .perform just like Resque does, and maybe open to customization like this: run FetchJob2, params: {some_flag: true, url: 'http://url.com'}, perform_method: :foobar

what do you guys think about that?

[pokonski]

In version 1.0 the jobs will no longer need to be inherited from Gush::Job, so stay tuned!

[pokonski]

Closing due to inactivity and general change of direction since my comment