Closes #317: Use megazord-gradle plugin to consume single Application….. #318
Conversation
|
This won't pass yet 'cuz the companion plugin at https://github.com/mozilla/megazord-gradle hasn't been published. I'm working on that and not sure why it's delayed. |
|
|
||
| System.setProperty("mozilla.appservices.fxaclient_ffi_lib_name", "reference_browser"); | ||
| System.setProperty("mozilla.appservices.logins_ffi_lib_name", "reference_browser"); | ||
| System.setProperty("mozilla.appservices.places_ffi_lib_name", "reference_browser"); |
There was a problem hiding this comment.
@ncalexan looks like ReferenceBrowserComposition.init executes the same three setProperty calls. Are these leftovers?
There was a problem hiding this comment.
The linter doesn't like the semicolons :).
There was a problem hiding this comment.
Yes to both. I'm actually going to take the plugin in a different direction, which I just sketched docs (before implementing) in mozilla/application-services#463. Perhaps you could take a look, @csadilek? It's pretty close to what's happening here, and shouldn't require significant changes. It's just simpler for the plugin implementation and better for supporting unit testing transparently.
There was a problem hiding this comment.
@ncalexan read through it. Looks good to me!
ncalexan
force-pushed
the
megazord
branch
2 times, most recently
from
5017dd4 to
5666249
Compare
|
@csadilek you looked at an earlier version of this (in a separate repo). I've put the plugin in appservices and reworked the internals significantly: it's simpler, you just declare the megazord you care about now. Can you take another look? Thanks! This is busted in the wild just due to needing some publications to go out, so this will require an app-services bump in a-c. I'll get that in place. |
|
Just an update on this: the plugin is published and a-s v0.12.0 is published. Waiting to bump a-c (ticket link to follow) and then update this PR. |
…n to consume single Application Services megazord native lib.
|
I force-pushed to include some version and name changes, but there's nothing substantially different. I'll inspect the try results and if this is green and has the correct libraries included, I'll land it. |
|
Ah, I can't merge. In any case, based on the logs above, I think this ready to merge. I filed #354 to allow more authoritative testing. |
csadilek
changed the title
csadilek
changed the title
|
Rebased and tested this locally. Let's land this, build failures are network issues.... |
… Services native lib.