Don't Load GVL v1 for TCF2 (+ TCF1 Cleanup) #1693
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hhhjort
reviewed
Feb 8, 2021
gdpr/vendorlist-fetching.go
Outdated
| latestVersion := saveOne(ctx, client, urlMaker(0, tcfSpecVersion), saver, tcfSpecVersion) | ||
| func preloadCache(ctx context.Context, client *http.Client, urlMaker func(uint16) string, saver saveVendors) { | ||
| latestVersion := saveOne(ctx, client, urlMaker(0), saver) | ||
| firstVersion := uint16(2) // The GVL for TCF2 has no vendors defined and is very unlikely to be used. Don't load it. |
There was a problem hiding this comment.
Incomplete comment. It is the first version of the GVL for TCF2 that has no vendors, not all TCF2 GVLs. :)
bsardo
reviewed
Feb 16, 2021
gdpr/vendorlist-fetching.go
Outdated
| latestVersion := saveOne(ctx, client, urlMaker(0, tcfSpecVersion), saver, tcfSpecVersion) | ||
| func preloadCache(ctx context.Context, client *http.Client, urlMaker func(uint16) string, saver saveVendors) { | ||
| latestVersion := saveOne(ctx, client, urlMaker(0), saver) | ||
| firstVersion := uint16(2) // The first version of the GVL for TCF2 has no vendors defined and is very unlikely to be used. Don't preload it. |
There was a problem hiding this comment.
Thoughts on calling this secondVersion instead of firstVersion and moving this comment to just above the for loop that follows this?
There was a problem hiding this comment.
How about firstVersionToLoad? That's a more a accurate description IMHO.
hhhjort
approved these changes
Feb 16, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partially addresses #1689 and separates the TCF1 vs TCF2 fetchers for easy removal of TCF2 in the near future and removal now for TCF1 url construction.