services.Issue: directly access configuration #978
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #979, #976, and #973.Rather than using the
origin
dictionary to pass configuration valuesfrom Service to Issue, give
Issue
direct access to theconfig
andmain_config
objects.There are several advantages to this approach:
conception. It's elimination means the (more straightforward)
conception of "config" can be transferred from the
Service
object.That's one fewer concept for developers to juggle, which is good for
bugwarrior core maintenance and for service development.
get_service_metadata
. This method's name also doesnot really convey it's purpose and I was unsurprised to find that
many services pass data through this method which is never actually
used by the
Issue
.Service
andIssue
. Changes that only involveIssue
and configuration no longer require meddling with
Service
, whichis just a middle man.
Service
baseclass and instances. The child classes were calling
self.get_issue_for_record
on the base class, which was callingself.get_service_metadata
on the child class. In some ways this isa re-iteration of point (2) -- this is a pattern that leads to
confusion and to me is a code smell.
The architectural trade-off is that
Issue
is now directly coupled toconfiguration, but in my opinion the previous indirection was merely a
vicarious coupling that doesn't provide much benefit any more. When this
design was first implemented, configuration was parsed in the
Service
class, so it made sense because it avoided duplication of parsing and
provided consistency; this is no longer a relevant consideration.
This is largely motivated by a desire to simplify services for #775.