Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

MyJob.enqueue Syntax #77

Closed
jduarte opened this issue May 29, 2014 · 6 comments
Closed

MyJob.enqueue Syntax #77

jduarte opened this issue May 29, 2014 · 6 comments

Comments

@jduarte
Copy link

jduarte commented May 29, 2014

Hi there,

I believe, to get a more consistent Rails-like syntax, that enqueue should have an options hash opposed to enqueue_in and enqueue_at methods.

So these would be valid method calls:

MyJob.enqueue record, :at => Date.tomorrow.now
MyJob.enqueue record, :in => 1.week

Since the given method definition is valid Ruby:

def enqueue(*args, options={})
  # ...
end

it seems to me that it's a most concise approach given Rails method definitions.

What do you think?

@dhh
Copy link
Member

dhh commented May 30, 2014

That's going to conflict with people who expect a hash of options to their own job, unfortunately. When the argument is first, it's unambiguous.

On May 30, 2014, at 1:15, José Duarte notifications@github.com wrote:

Hi there,

I believe, to get a more consistent Rails-like syntax, that enqueue should have an options hash opposed to enqueue_in and enqueue_at methods.

So these would be valid method calls:

MyJob.enqueue record, :at => Date.tomorrow.now
MyJob.enqueue record, :in => 1.week
Since the given method definition is valid Ruby:

def enqueue(*args, options={})

...

end
it seems to me that it's a most concise approach given Rails method definitions.

What do you think?


Reply to this email directly or view it on GitHub.

@cristianbica
Copy link
Member

What would work is

MyJob.enque in: 5.hours, args:[1, 2, option: 3]
MyJob.enque at: 5.hours.from_now, args:[1, 2, option: 3]
MyJob.enque args:[1, 2, option: 3]

but I personally prefer the 3 methods enqueue/enqueue_in/enqueue_at as it's cleaner

@seuros
Copy link
Member

seuros commented May 30, 2014

👍 for staying with the current syntax only.

@Alexander-Senko
Copy link

What about this?

MyJob.at(Date.tomorrow).enqueue record
MyJob.in(1.week).enqueue record

Or this (bad for default usage)?

MyJob.enqueue(record).now
MyJob.enqueue(record).at Date.tomorrow
MyJob.enqueue(record).in 1.week

@dhh
Copy link
Member

dhh commented May 30, 2014

I don’t think that’s an improvement on the there separate enqueue methods.

On May 30, 2014, at 5:31 PM, Alexander Senko notifications@github.com wrote:

What about this?

MyJob.at(Date.tomorrow).enqueue record
MyJob.in(1.week).enqueue record
Or this (bad for default usage)?

MyJob.enqueue(record).now
MyJob.enqueue(record).at Date.tomorrow
MyJob.enqueue(record).in 1.week

Reply to this email directly or view it on GitHub.

@jduarte
Copy link
Author

jduarte commented May 31, 2014

Given the job arguments issue I believe the current syntax is the most appropriate.

@dhh dhh closed this as completed Jun 5, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants