This differential introduces isStaffRelease in PlatformSpecificTools class. Method is callable from C++, Java and Objective-C code including NSE code. Ther reason for this functionality is that currently only JavaScript code can be landed
directly to the master and switched on only for staff users (using native/staff-utils.js). If developer wants to introduce some changes in native languages that sholud be active only for staff users they have to directly colaborate with release-maker.
With changes introduced in this diff developer can land staff-only changes directly to master by hiding them behind PlatformSpecificTools#isStaffRelease and release-maker only has to change two methods to return true inseated of false.
Details
Details
- Apply this diff: https://gist.github.com/marcinwasowicz/38435924b5850515649253a2e490572e (CommCoreModule, CommNotificationsHandler, NSE)
- Build iOS and Android app.
- Log in on both - observe logs from CommCoreModule.
- Send notification to both: Observe logs from CommNotificationHandler and NSE (this one requires usage of console app)
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- marcin/eng-4453
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Is it necessary to have two places that we need to change? Could the isStaffRelease function have been placed in a shared CPP file, so it only needs to be updated in one place for both iOS and Android?
Comment Actions
Talked about this with @marcin – it should be possible to keep this flag in just one place that is accessible from iOS and Android
Comment Actions
- Implement the method in C++ and call from Java instead of implementing in Java and exporting to C++
| native/cpp/CommonCpp/Tools/StaffUtils.cpp | ||
|---|---|---|
| 5 ↗ | (On Diff #29946) | So this is the value that'll need to be flipped for staff releases, correct? We should probably document in the Mobile releases doc: https://www.notion.so/commapp/Mobile-releases-62f3ed6eed33413f9e2b09fcfc797e82 |
Comment Actions
Shouldn't there be some change to the .pbxproj file for iOS? Are the new iOS files showing up in Xcode?
Comment Actions
Requesting changes for .pbxproj. Adding @jon to confirm CMakeLists.txt updates are correct
| native/cpp/CommonCpp/Tools/StaffUtils.cpp | ||
|---|---|---|
| 5 ↗ | (On Diff #29946) |
Exactly - staff releases must have this value set to true to unlock staff-only C++/Java/Objective-C code.
Looks like there is no entry for staff-utils.js file that also gets updated on every staff release. Perhaps both of those places could be mentioned together in the same section? |