Move shared BaseExecution concerns into the base class.
#1009
Conversation
Parent/base class for `Job and Execution` is `BaseExecution`, both of these child classes include same concerns: `Lockable, Filterable, Reportable`. This commit moves inclusion of these concerns to the base class `BaseExecution`
| include Filterable | ||
| include Lockable | ||
| include Reportable |
There was a problem hiding this comment.
Would love if I could group this; include Filterable, Lockable, Reportable
but rubocop is not allowing it :)
There was a problem hiding this comment.
Seems like a standard style; Rails uses it too. My digging surfaced this, but not the best explanation :-)
There was a problem hiding this comment.
I agree definitely not worth changing this rule, it just looked nicer in this particular situation to me :)
I think this rule is on by default because of:
- Git diffing
- Error tracing - line number, if there are multiple mixins on 1 line could be harder to trace
|
This looks good. Thank you! btw, there's going to be some major refactoring in GoodJob v4 that will do away with the BaseExecution. There will just be |
I'm willing to work on this, if you haven't done it already :) |
Parent/base class for
Job and ExecutionisBaseExecution, both of these child classes include same concerns:Lockable, Filterable, Reportable.This commit moves inclusion of these concerns to the base class
BaseExecution