Rather than using the origin dictionary to pass configuration values
from Service to Issue, give Issue direct access to the config and
main_config objects.
There are several advantages to this approach:
"Origin" is an unclear name of which a developer has to build a
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.
Elimination of get_service_metadata. This method's name also does
not 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.
Decoupling Service and Issue. Changes that only involve Issue
and configuration no longer require meddling with Service, which
is just a middle man.
Elimination of bidirectional communication between Service base
class and instances. The child classes were calling
self.get_issue_for_record on the base class, which was calling
self.get_service_metadata on the child class. In some ways this is
a 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 to
configuration, 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.
Based on #979, #976, and #973.Rather than using the
origindictionary to pass configuration values from Service to Issue, giveIssuedirect access to theconfigandmain_configobjects.There are several advantages to this approach:
Serviceobject. 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 does not 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 theIssue.ServiceandIssue. Changes that only involveIssueand configuration no longer require meddling withService, which is just a middle man.Servicebase class and instances. The child classes were callingself.get_issue_for_recordon the base class, which was callingself.get_service_metadataon the child class. In some ways this is a 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
Issueis now directly coupled to configuration, 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 theServiceclass, 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.