Move SiriusFactory to top-level api package.#93
Move SiriusFactory to top-level api package.#93comcast-jonm wants to merge 2 commits intoComcast:masterfrom
Conversation
This is a re-spin of Comcast#27 but also incorporates the intent of Comcast#68. This should basically mean that a client of the Sirius library ought only to directly use classes, objects, and traits from the `com.comcast.xfinity.sirius.api` package, and that, going forward, those are the files that must retain backwards compatibility. The main changes involved are: * Additional extension methods for 1.3 are now collected in the Sirius1Dot3 trait that extends the existing Sirius trait. * Existing SiriusFactory was hoisted up to api package and now returns a Sirius1Dot3 instead of a SiriusImpl. Clients ought to be depending on a trait rather than a concrete class here. * There is still a SiriusFactory in the old location, but it just delegates to the one in the new location, then downcasts the return result to a SiriusImpl. This SiriusFactory is marked as deprecated.
|
This differs from #68 in that it moves the factory to the top-level api package, whereas I had moved it to just the root sirius package, in part because the use of an 'api' package seems somewhat anachronistic to me. That may be a separate discussion, but it seems worth deciding once and for all whether we need a separate 'api' package. |
|
@smuir : That's a fair point. I did the minimal change of just moving one class to put everything together. You're probably right that the whole thing could be moved up a level, although I don't personally mind having an 'api' package, since that's what it is. At least with this change, someone wanting to upgrade probably only has to change a single import statement in a single file, since none of the other classes change. |
|
I'm not going to argue any further against the 'api' package, so this has my 👍 That doesn't mean I like it, but I can live with it. |
|
Ah, the api package... Yeah, IIRC that was largely an oversight, followed by inertia, followed by concerns about moving message classes around and how remote nodes on different versions would handle it. That being said it would be nice to rearrange, it just might take a bit of effort. |
3 participants
This is a re-spin of #27 but also incorporates the intent of #68.
This should basically mean that a client of the Sirius library
ought only to directly use classes, objects, and traits from the
com.comcast.xfinity.sirius.apipackage, and that, goingforward, those are the files that must retain backwards
compatibility.
The main changes involved are:
Sirius1Dot3trait that extends the existingSiriustrait.SiriusFactorywas hoisted up toapipackage and nowreturns a
Sirius1Dot3instead of aSiriusImpl. Clients oughtto be depending on a trait rather than a concrete class here.
SiriusFactoryin the old location, but itjust delegates to the one in the new location, then downcasts
the return result to a
SiriusImpl. ThisSiriusFactoryismarked as deprecated.