UID2-2832 change domain name to domain or app name #59
Conversation
This file contains 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
…nd BidstreamClient decrypt methods
caroline-ttd
commented
Apr 29, 2024
src/UID2.Client/Site.cs
Outdated
| @@ -10,7 +10,7 @@ internal class Site | |||
| public Site(int id, IEnumerable<string> domainNames) | |||
| { | |||
| Id = id; | |||
| _domainNames = new HashSet<string>(domainNames, StringComparer.OrdinalIgnoreCase); | |||
| _domainNames = new HashSet<string>(domainNames); | |||
There was a problem hiding this comment.
I did not change the variable name and method name inside the site class because the json format site_data has the field called domain_names
sunnywu
reviewed
May 1, 2024
jon8787
reviewed
May 1, 2024
| @@ -16,7 +16,7 @@ public enum DecryptionStatus | |||
| /// DSPs are still expected to check their records for user opt out, even when this status is not returned | |||
| /// </summary> | |||
| UserOptedOut, | |||
| DomainNameCheckFailed, | |||
| DomainOrAppNameCheckFailed, | |||
There was a problem hiding this comment.
will this break consuming code? such as found in gitlab
There was a problem hiding this comment.
there will be changes required there as we upgrade it to use this version of SDK. tho probably we should check with @vishalegbert-ttd @gmsdelmundo if we change this status, do we need to make further changes in other source codes e.g. spark jobs
There was a problem hiding this comment.
From the discussion in the slack, we can keep this change.
sunnywu
approved these changes
May 2, 2024
jon8787
approved these changes
May 2, 2024
caroline-ttd
deleted the
ccm-UID2-2832-change-domain-name-to-domain-or-app-name
branch
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.
domainNametodomainOrAppNameand correspond methods, tests and DecryptionStatus' name.