-
Notifications
You must be signed in to change notification settings - Fork 887
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
Ad matching for ads that are not categorized #3158
Conversation
@jsecretan @lukemulks The above issue is development complete, we need a list of untargeted regions for initial release. Thanks |
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.
iOS changes look good
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.
LGTM, one minor comment
vendor/bat-native-ads/README.md
Outdated
@@ -62,7 +62,7 @@ void ChangeLocale( | |||
|
|||
`ClassifyPage` should be called when a page has loaded in the current browser tab, and the HTML is available for analysis |
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.
Function name in comment needs an update...
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.
Fixed
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.
LGTM
…ombia, Denmark, Ecuador, Israel, India, Italy, Korea, Mexico, Netherlands, Peru, Philippines, Poland, Sweden, Singapore, Venezuela and South Africa to Ads
Added support for Argentina, Austria, Brazil, Switzerland, Chile, Colombia, Denmark, Ecuador, Israel, India, Italy, Korea, Mexico, Netherlands, Peru, Philippines, Poland, Sweden, Singapore, Venezuela and South Africa to Ads (untargeted) |
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.
LGTM
Fixes brave/brave-browser#5183
Requires #3171
Requires #3172
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
Reviewer Checklist:
After-merge Checklist:
changes has landed on.