Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request removes the AnalyticsGroupJoin method and its associated UI button from the Analytics sample application. The reviewer suggests retaining this method to maintain a comprehensive example for users, proposing an update to use a string literal for the group ID parameter instead of removing the functionality entirely.
I am having trouble creating individual review comments. Click here to see my feedback.
analytics/testapp/Assets/Firebase/Sample/Analytics/UIHandler.cs (99-105)
Instead of removing the AnalyticsGroupJoin method, consider updating it to use the string literal "group_id". The EventJoinGroup event is still a standard recommended event, and keeping this example is beneficial for users of the quickstart to understand how to log group-related events.
public void AnalyticsGroupJoin() {
// Log an event with a string parameter.
DebugLog("Logging a group join event.");
FirebaseAnalytics.LogEvent(FirebaseAnalytics.EventJoinGroup, "group_id",
"spoon_welders");
}
I removed all ParameterGroupID references within the Quickstart project for Analytics as it's already deprecated (see here) and was causing errors :
