Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-26261] Android: Compile with API Level 28 (Android P) #10307

Merged
merged 3 commits into from Sep 7, 2018

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Sep 4, 2018

JIRA:
https://jira.appcelerator.org/browse/TIMOB-26261

Summary:

  • Modified Titanium to compile against API Level 28 (Android P).
  • Does not target API Level 28 by default yet.
    • To be done once all Android P breaking changes have been resolved.
    • App dev can opt-in to targeting API Level 28 via "tiapp.xml" <uses-sdk/> setting.
  • Updated Android build-tools max version from 27.x to 28.x.
  • Updated Android NDK max version from r16c to r17b.

Test:
Build and run KitchenSinkV2 on Android 9.0 and Android 4.1. Traverse app's UI features and verify that they work.

- Does not "target" API Level 28 by default yet. Will do so once all breaking-changes have been resolved.
@sgtcoolguy
Copy link
Contributor

So, technically speaking bumping the min node version is a "breaking change" and would need to go in a major release (like 8.0.0).

However, I think we kind of already have unofficially bumped the node minimum required in our toolset anyhow? I know Chris Barber wants it to be like Node 8.11 which is quite new, for appcd.

But I'm not sure who's keeping track of our broad-based Node min supported versions - it'd be good to ensure they're aligned between the SDK and all our 1st-party npm packages that we use in the CLI/SDK.

@mukherjee2 @eric34 Do we have an official Node minimum we're shooting for across the board? If we look at https://github.com/nodejs/Release I think we should be trying to support 6.x - 10.x right now.

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CR: PASS

@jquick-axway
Copy link
Contributor Author

@sgtcoolguy, right, I was told verbally that our min was 8.0. And QE has only been testing with 8.x as well. So, I got the impression that someone forgot to bump up the node versions in our "package.json".

We document the min as 8.0 here...
https://docs.appcelerator.com/platform/latest/#!/guide/Titanium_Compatibility_Matrix-section-src-29004837_TitaniumCompatibilityMatrix-Node.js

But we also document it as 6.0 in the same doc below. Although from a customer perspective, I think this would be confusing and I think we should only show one min version (keep the 8.0 one).
https://docs.appcelerator.com/platform/latest/#!/guide/Titanium_Compatibility_Matrix-section-src-29004837_TitaniumCompatibilityMatrix-Node.js.1

How about this. I'll change the min version back to what it was until we have consensus from the rest of the team.

- Waiting for consensus from team on what min version should be.
@lokeshchdhry
Copy link
Contributor

FR Passed.

Ran KS2 on API 28 & API 16 & it seems to work fine.

Studio Ver: 5.1.1.201809051655
SDK Ver: 7.5.0 local build
OS Ver: 10.13.5
Xcode Ver: Xcode 9.4.1
Appc NPM: 4.2.13
Appc CLI: 7.0.6
Daemon Ver: 1.1.3
Ti CLI Ver: 5.1.1
Alloy Ver: 1.13.2
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 10.0.2
Emulator: Android API28, Android API16

@build
Copy link
Contributor

build commented Sep 7, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

@hansemannn hansemannn merged commit 8909105 into tidev:master Sep 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants