Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why does jobs need to be passed to queue? #695

Open
fo-fo opened this issue Oct 31, 2021 · 6 comments
Open

Why does jobs need to be passed to queue? #695

fo-fo opened this issue Oct 31, 2021 · 6 comments

Comments

@fo-fo
Copy link

fo-fo commented Oct 31, 2021

The documentation has the following blurb:

new queue requires only the "queue" variable to be set. You can also pass the jobs hash to it.

What happens if I don't pass jobs? Why would I pass it if it's optional?

I was looking at the code (queue.ts), and it looked like the job's plugins would not run if I don't pass jobs to the queue (as const job = this.jobs[func]; would be undefined), is this intentional?

@evantahler
Copy link
Member

It's the other way around I think - if you don't pass jobs, queue.enqueue(), RunPlugins() would always succeed because the job couldn't be found.

I'll update the docs to to say:

new queue requires only the "queue" variable to be set. If you intent to run plugins before or after enquing a job, you should also pass the jobs hash to it.

@fo-fo
Copy link
Author

fo-fo commented Nov 2, 2021

It's the other way around I think - if you don't pass jobs, queue.enqueue(), RunPlugins() would always succeed because the job couldn't be found.

That's what I meant – the plugins set for the job would never run unless jobs is passed to the queue. Changing the docs sounds like a good idea, although in my mind it almost sounds like it should be a mandatory parameter, because the behavior without it is quite surprising in my opinion.

@evantahler
Copy link
Member

Ah, understood!
Please send in a PR with your suggested change, and we can discuss it there!

@evantahler
Copy link
Member

Ping! checking in @fo-fo - do you need any help with that PR?

@fo-fo
Copy link
Author

fo-fo commented Nov 25, 2021

Ping! checking in @fo-fo - do you need any help with that PR?

Hey, sorry, I haven't had time to do it (yet at least). Actually it's not a big deal to me whether the documentation gets changed, although I appreciate you verifying my assumption about how it works!

@evantahler
Copy link
Member

I'd be very happy with the update if you could make it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants