Feature/google analytics surrogates #167
Conversation
brindy
left a comment
brindy
left a comment
There was a problem hiding this comment.
Looks good! Just needs to be named more generically. A few comments inline.
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.duckduckgo.app.analyticsSurrogates |
There was a problem hiding this comment.
Not keen on the name. Is this analytics as in google? The surrogates is more generic than that, it's just that's the main one we're targeting now.
Also a personal thing, but really dislike camel naming in packages but that's because I'm used to the Java conventions, though it seems Kotlin allows it (but discourages two words in a package name if possible).
Just surrogates would do imho :)
For reference:
There was a problem hiding this comment.
Renaming to simply surrogates
| import org.junit.Before | ||
| import org.junit.Test | ||
|
|
||
| class AnalyticsSurrogatesLoaderTest { |
There was a problem hiding this comment.
Likewise, I think should drop Analytics in any of the names through out. (I won't flag any others)
There was a problem hiding this comment.
Will replace AnalyticsSurrogate with ResourceSurrogate throughout
| @UiThreadTest | ||
| @Before | ||
| fun setup() { | ||
| MockitoAnnotations.initMocks(this) |
There was a problem hiding this comment.
We seem to be mixing and matching approaches. Nothing to do here, but we should all perhaps have a chat about what the standard approach should be (I prefer just creating them directly without annotations or using initMocks personally).
| fun before() { | ||
| mockDao = mock() | ||
| testee = HttpsUpgrader(mockDao) | ||
| testee = HttpsUpgraderImpl(mockDao) |
There was a problem hiding this comment.
The great implementation name debate. But nothing to change here, another one to discuss probably.
| @@ -0,0 +1,107 @@ | |||
| # To neutralize GA scripts. The goal is to provide the minimal API | |||
There was a problem hiding this comment.
So these files are being served up by an endpoint for licensing issues. To avoid any problems can you create mock versions of these files, please?
Co-authored-by: David González <malmstein@gmail.com>
2 participants
Asana Issue URL: https://app.asana.com/0/488551667048375/518654221411512
Description
Implement Google Analytics surrogates to prevent some sites from breaking
Steps to Test this PR:
If the site successfully loads, this is a good indicator the surrogate is working. For comparison, do the same in
developbranch and confirm it doesn't work there.