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

refactor(android): rewrite old python scripts in Node.js #11723

Merged
merged 2 commits into from Jun 4, 2020

Conversation

sgtcoolguy
Copy link
Contributor

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

Description:
In working on PR #11693, I was doing a lot of work on our build/startup for iOS/Android and ran across our usage of these very old scripts to generate under the hood files for Android. We already had some code for doing the C string byte array baking stuff from titanium_mobile_windows, so I copied it over and the work grew to get rid of these last python scripts.

The generation of the bootstrap.js was partially ported from the old python script, but I removed some logic that was unused in the SDK's modern/existing bindings (i.e. having to merge multiple definitions of the same module/proxy together).

Note that there's an nearly-identical process in the native module template: generate-cpp-header-files.js. That implementation intentionally doesn't use npm packages to do it's work. Long-term it should simply point at the SDK to do the work for it (i.e. these new Node scripts should be "formalized" and shipped in the built SDK and that file should just mostly defer to them)

@build build added this to the 9.1.0 milestone May 20, 2020
@build
Copy link
Contributor

build commented May 20, 2020

Messages
📖 👍 Hey!, You deleted more code than you added. That's awesome!
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖

✅ All tests are passing
Nice one! All 6648 tests are passing.
(There are 709 skipped tests not included in that total)

Generated by 🚫 dangerJS against f7bd4f6

Copy link
Contributor

@jquick-axway jquick-axway 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

We should test this PR with the last released encrypted DB module for backward compatibility just-in-case.

This is because that module's generated "bootstrap.js" also pushes an entry into to the core invocationAPIs array. From looking at the module's generated bootstrap, it "should" be fine since it pushes to the array directly via its own addInvocationAPI() function.

@sgtcoolguy
Copy link
Contributor Author

I tested running the appcelerator.encrypteddatabase module unit tests on Android with this SDK and that passed (although the tests are not very exhaustive, just checking we can load the module and checking the db read/write integer boundaries).

@sgtcoolguy sgtcoolguy merged commit 1a87e4f into tidev:master Jun 4, 2020
@sgtcoolguy sgtcoolguy deleted the TIMOB-27719 branch June 4, 2020 19:32
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

3 participants