Remove AnswerAnalysisJob in favour of method#763
Conversation
jackbot
force-pushed
the
answer-analysis-job-remove
branch
2 times, most recently
from
f42d7d1 to
571d5fa
Compare
| @@ -0,0 +1,6 @@ | |||
| module AnswerAnalysis | |||
| def self.enqueue_async_analysis(answer) | |||
There was a problem hiding this comment.
i wonder if this should live in the AutoEvaluation module instead?
There was a problem hiding this comment.
I'd probably suggest it feels better here given the classes it calls are already under the AnswerAnalysis namespace.
I'd probably suggest we move this module to lib/ though as this isn't a model and we use this namespace in more than just models.
There was a problem hiding this comment.
Not one for this PR, but maybe we should consider renaming lib/auto_evaluation to lib/answer_analysis so it's consistent. I'm not sure we're gaining anything from them being different atm.
There was a problem hiding this comment.
Thats a reasonable shout. At the moment I'm ok with it as I think of AutoEvaluation as just one aspect of analysis, but given we've got the TopicTagger in there we're probably already using both namespaces interchangeably.
There was a problem hiding this comment.
Coo, I've moved the module to the lib directory.
| @@ -0,0 +1,6 @@ | |||
| module AnswerAnalysis | |||
| def self.enqueue_async_analysis(answer) | |||
There was a problem hiding this comment.
I'd probably suggest it feels better here given the classes it calls are already under the AnswerAnalysis namespace.
I'd probably suggest we move this module to lib/ though as this isn't a model and we use this namespace in more than just models.
Rather than enqueuing a background job just to enqueue more background jobs, we can just do all this in a method instead.
jackbot
force-pushed
the
answer-analysis-job-remove
branch
from
571d5fa to
7e870c4
Compare
4 participants
Rather than enqueuing a background job just to enqueue more background
jobs, we can just do all this in a method instead.