-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Remove unused analytics #2964
Remove unused analytics #2964
Conversation
eeb0b74
to
2d3c40f
Compare
83fd2db
to
c3fc05b
Compare
c3fc05b
to
4285854
Compare
@@ -1220,12 +1217,6 @@ public InstanceMetadata getSubmissionMetadata() { | |||
// timing element... | |||
v = e.getChildrenWithName(AUDIT); | |||
if (v.size() == 1) { | |||
Collect.getInstance().getDefaultTracker() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would keep this since it's an evolving feature. It might also be good to log when location is turned on.
Can you say more about audit logging not being actionable? It feels to me like it's important to know whether it's something to keep investing in. I suppose if that's already decided, the logging doesn't matter. |
I checked in with @yanokwa in person and he said "people who use the audit log say it's a game changer so we're going to continue investing in it regardless." 👍 |
Could we remove logs about using the splash screen as well? Here I noticed they cause crash sometimes. I think those we have collected during last two months are enough to make a decision whether we can remove the screen or not. |
cf8feac
to
ffba655
Compare
ffba655
to
134b705
Compare
Tested with success. Verified on Android: 4.2, 4.4, 5.1, 6.0, 7.0, 8.1 I focused the most on forms:
@opendatakit-bot unlabel "needs testing" |
Closes #2916
What has been done to verify that this works as intended?
Searched codebase for all
setCategory
and removed the calls to the events that generate the most hits (ExternalData, Itemset) or are top events and are not actionable (AuditLogging).I did not verify on a device or Google Analytics that these events are not being sent.
Why is this the best possible solution? Were any other approaches considered?
Narrowest change possible
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
There should be no user-facing changes
Do we need any specific form for testing your changes? If so, please attach one.
No
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No
Before submitting this PR, please make sure you have:
./gradlew checkAll
and confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTest
locally.