Add cmake code for performance module #707
Conversation
|
Thanks for sending this! Since Performance is not officially supported in the C++ SDK yet, I'd like to ensure that this is not included in builds by default, so could you change the default setting to reflect that? Developers who want to include it can specify -DFIREBASE_INCLUDE_PERFORMANCE=TRUE. Also, could you ensure that the stub build compiles on at least one desktop platform (e.g. Mac)? |
| @@ -52,6 +52,9 @@ option(FIREBASE_INCLUDE_INSTALLATIONS | |||
| option(FIREBASE_INCLUDE_MESSAGING | |||
| "Include the Firebase Cloud Messaging library." | |||
| ${FIREBASE_INCLUDE_LIBRARY_DEFAULT}) | |||
| option(FIREBASE_INCLUDE_PERFORMANCE | |||
| "Include the Firebase Performance library." | |||
| ${FIREBASE_INCLUDE_LIBRARY_DEFAULT}) | |||
There was a problem hiding this comment.
Could you make the default setting false for this? Since it's not officially supported, users should have to explicitly opt in to include this module (at least for now).
| @@ -14,6 +14,7 @@ target 'GetPods' do | |||
| pod 'Firebase/Functions', '8.8.0' | |||
| pod 'Firebase/Installations', '8.8.0' | |||
| pod 'Firebase/Messaging', '8.8.0' | |||
| pod 'Firebase/Performance', '8.8.0' | |||
There was a problem hiding this comment.
Can you update this to the latest version, 8.9.1? (also resolve the merge conflict in this file)
| @@ -0,0 +1,143 @@ | |||
| # Copyright 2019 Google | |||
There was a problem hiding this comment.
nit: 2021, as that was when the file was created
| @@ -17,6 +17,7 @@ include ':app', | |||
| ':installations', | |||
| ':messaging', | |||
| ':messaging:messaging_java', | |||
| ':performance', | |||
There was a problem hiding this comment.
I'd also like to ensure that this isn't included in Android builds by default; I think the simplest way is to leave this line commented-out? (You can add a note to the readme about how to enable it.)
Description
As discussed in #671 this PR provides some basic CMake and gradle code to compile the performance module.
Testing
I have currently only tested the Android and iOS binaries within our internal project.
Type of Change
Place an
xthe applicable box:Notes
Release Notessection ofrelease_build_files/readme.md.