jessetane / queue

Asynchronous function queue with adjustable concurrency
MIT License
764 stars 66 forks source link

Emit a start event when a job begins to process #58

Closed joelgriffith closed 5 years ago

joelgriffith commented 5 years ago

Really appreciate this module and all the work that has gone into it. It's a great example of a simple library that does one thing really well, and is readable. Thanks so much!

This adds a simple start event when a job begins (and exposes that job fn to any listeners). This is useful, in my case, where we append properties to the job so that we can track things like duration in the queue, when it started etc. Adds a test case as well.

coveralls commented 5 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 8f08ea77609d7cea9d1ada79ac145bc87b4a55b6 on joelgriffith:master into 244cb2f8c09be96c103e4557b58066694d8a6aea on jessetane:master.

jessetane commented 5 years ago

Thanks for the patch. Generally in favor but a couple thoughts - mainly, if the job calls back synchronously, I think the success event will fire before the start event, which seems a bit odd.. I wonder if emitting start before the job actually runs works and is ok for you?

Also, in the test, what do you think about moving the event check to the beginning (you could remove the handler with the job) so that we don't need to add the extra call to start() and everything can run in one go?

joelgriffith commented 5 years ago

Thanks for the feedback, adjusted!