-
Notifications
You must be signed in to change notification settings - Fork 92
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
Refactoring/Improvement #230
Conversation
…cated 2. `userKeywords` was renamed to `getUserKeywords`
2. `CollectionExtensionTest.swift` was moved to `UnitTests` group
2. refactoring and improvements of UI tests
Hi @yoalex5, can you resolve the merge conflict on this branch? |
*/ | ||
@available(*, deprecated, message: "Please use Targeting.shared.addUserKeyword() method instead") | ||
@available(*, deprecated, message: "Please use Targeting.addUserKeyword() method instead") | ||
public func addUserKeyword(key: String, value: String) { |
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.
if key is omitted, why not change the API to be just passing one String, same as Android?
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.
yes, the new targeting keywords should not utilize a key. It should be a static string. I believe this may be a mistake.
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.
Hello @anwzhang,
if key is omitted, why not change the API to be just passing one String
I did not do it because it is a public API and I want to have a backward compatibility
same as Android
Android version has the same changes
https://github.com/prebid/prebid-mobile-android/blob/67df7e2f9be348f33147681454a1e94f66ec1a97/PrebidMobile/API1.0/src/main/java/org/prebid/mobile/AdUnit.java#L150
If you wish I can change PRs for iOS and Android what will entail breaking changes as a result we have to change a major version
# Conflicts: # src/PrebidMobile/PrebidMobile/RequestBuilder.swift # src/PrebidMobile/PrebidMobileTests/FetchingLogictests/RequestBuilderTests.swift
@anwzhang, I have resolved the merge conflicts |
@anwzhang, could you please review it ? |
These changes generally refactor the FirstPartyData PR. Plus minor changes
AdUnit.UserKeyword
usesTargetingParams.UserKeyword
inside. The parameterkey
will be omitted