API Changes

[yelled3]

We still have to discuss about a good API.

Topics at hand:

• configure ORM in initializer (+ default to ActiveRecord) so there won't be a need for namespace. see: https://github.com/yelled3/sidekiq-activerecord/pull/1#commitcomment-6397008
• class/method naming
• remove sidekiq_ prefix from options. see: https://github.com/yelled3/sidekiq-activerecord/pull/1#issuecomment-43726149

@seuros did I forget anything?

I'm open to suggestions.

@mperham /cc

[seuros]

I still think that initializer is a bad idea. Namespacing is the cleanest way to do it.

• We should be able to see a worker and know which extension it using.
• I don't like that we writing in Sidekiq namespace while the gem is Sidekiq::ActiveRecord according do the naming.
• With pry, you can easily do a ls Sidekiq::ActiveRecord and see it submodule.
• It will be more hard to test all plugins together.
• Not all frameworks have initializer folder.

[yelled3]

The main issue I have with this, is the duplication of logic.

think about TaskWorker; the only thing that is ORM related is:

def fetch_model(identifier)
      model_class.find_by(identifier_key => identifier)
end

everything else, is pure logic which I don't want to duplicate.
that why, I want to have a base class that handles the logic, and sub-module (per-ORM) that implements the ORM related parts. (which is a single method in this case)

notice that the current task_worker_spec, doesn't directly tests anything that's really ORM related. so it should be fairly easy to refactor.

We should be able to see a worker and know which extension it using.

in most cases, an application uses a single ORM. and the default ORM is obvious to all.
in edge cases where you want to work with another, it should also be clear if we do:

class UserTaskWorker
  include Sidekiq::TaskWorker
  sidekiq_orm :mongoid # override the default per worker
end

again, I really think this will almost NEVER happen...

I don't like that we writing in Sidekiq namespace while the gem is Sidekiq::ActiveRecord according do the naming.

I agree. that's a good point;
if we go down this road, we'll need to rename the repo to something else.

BTW,
@seuros you're now a collaborator.
welcome :-)

[seuros]

Then let author another gem sidekiq-orm and have sidekiq-activerecord, sidekiq-sequel and sidekiq-datamapper inherit the logic from it.

[seuros]

I created an org and added you as owner. Let move this gem there and start implementing. WDYT ?

[yelled3]

sounds great.

[yelled3]

@seuros I renamed the repos to include sidekiq- prefix.
also, sidekiq-orm-core is the core logic.
and sidekiq-orm will be the meta-gem. (same as https://github.com/rspec/rspec )

[seuros]

Fine

[seuros]

@yelled3 , what do you mean with meta-gem ? I don't see that with sidekiq-orm.
Will it load all orm or just the core ? if just the core, then why another gem ?

We probably can use it to write test on it

[yelled3]

@seuros that's correct. I haven't added anything yet, but it should be exactly like rspec/rspec:

https://github.com/rspec/rspec/blob/master/Gemfile#L5-L7
and:
https://github.com/rspec/rspec/blob/master/rspec.gemspec#L33-L39