chore(android): include keyman-engine as sub-project - #16246
chore(android): include keyman-engine as sub-project#16246ermshiperete wants to merge 2 commits into
Conversation
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
385381e to
8b31f67
Compare
Adding keyman-engine as a sub-project to KMAPro as well as the sample apps and KeyboardHarness allows Android Studio to load both at the same time which makes it easier to debug and develop. Test-bot: skip
8b31f67 to
d74e2cd
Compare
jahorton
left a comment
There was a problem hiding this comment.
I love doing it for our main Keyman app, no question. Not sure about doing it for sample apps, though - external 3rd parties may not wish to link Keyman Engine for Library as a full-on subproject.
Good point. What about we'd add the non-subproject option as comment? I suggested the changes for KMSample1, but the same would have to be done for KMSample2 as well. |
I feel like @mcdurdin would want to chime in on this. |
|
I'm not sure about this. It's convenient for the development cycle but we need to make sure we treat keyman-engine as completely independent of keyman-app, and when editing them together we are more likely to accidentally modify API surfaces of keyman-engine without realizing. IMO the friction is helpful as a reminder: we need to be very deliberate about changes to keyman-engine. The sample apps should still be linking the .aar and not using a subproject approach -- we want to encourage the use of the .aar. |
So... should we split the iOS engine + app workspace into two separate workspaces? Right now, they're unified - just like this change would do for Android. |
Willing to discuss. Does add friction... |
Yep, I agree, that makes sense.
On this I disagree. We shouldn't make our lives harder than it has to be. Yes, we need to make sure we treat keyman-engine as completely independent of keyman-app, but that should be enforced by checks during either local builds or CI, not by a development workflow that requires manual steps and raises the bar for team members not very familiar with Android development. We might not have those checks in place, so I'm fine with deferring merging this PR until those are implemented, but I think the general direction should be to make our lives easier, not harder. |
Sample apps should not reference the project but the .aar instead.
Adding keyman-engine as a sub-project to KMAPro as well as the sample apps and KeyboardHarness allows Android Studio to load both at the same time which makes it easier to debug and develop.
Test-bot: skip