Skip to content
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

Add support for targetPercentile RMF rule parameter #809

Merged
merged 13 commits into from
May 14, 2024

Conversation

samsymons
Copy link
Contributor

@samsymons samsymons commented May 5, 2024

Please review the release process for BrowserServicesKit here.

Required:

Task/Issue URL: https://app.asana.com/0/0/1207234800675204/f
iOS PR: duckduckgo/iOS#2824
macOS PR: duckduckgo/macos-browser#2772
What kind of version bump will this require?: Major

Description:

This PR adds support for the new targetPercentile option in RMF rules. This allows us to specify a percentage of users for whom the rule matches.

Steps to test this PR:

  1. See client PRs

OS Testing:

  • iOS 14
  • iOS 15
  • iOS 16
  • macOS 10.15
  • macOS 11
  • macOS 12

Internal references:

Software Engineering Expectations
Technical Design Template

samsymons added 6 commits May 3, 2024 17:53
We used to only care about attributes and their rule ID, but now we’re starting to add additional properties so it’s time to turn rule into a class.
@samsymons samsymons requested a review from amddg44 May 5, 2024 19:07
@samsymons samsymons marked this pull request as ready for review May 6, 2024 02:48
let rules: [Int: [MatchingAttribute]]
let rules: [RemoteConfigRule]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is technically less efficient than the old implementation, but a bit simpler. We have so few rules at one time that the difference here is negligible, but let me know if you feel otherwise.

}

public struct RemoteConfigTargetPercentile {
let before: Float?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the percentile that will be sampled before we apply the attributes. The alternative here is that we do the sampling after matching attributes, but it was agreed that we'll only support the before case for now.

* main:
  Enable field validation for Sync payloads (#804)
* main:
  Enable gzip compression for Sync PATCH payloads (#803)
  autofill: send current language in runtime config (#808)
  Password manager survey update (#811)
  on iOS allow bookmarks in top hits (#812)
  Rename tests to avoid conflicts (#813)
  Bat.js fix (fix for installing content blocking rules multiple times) (#779)
  Pixels automatic naming prefixing fixed (#810)
Copy link
Contributor

@amddg44 amddg44 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Have tested thoroughly over the last couple of days and this all looks really great; nice work @samsymons

* main:
  Support new privacy dashboard source (#798)
@samsymons samsymons merged commit 6568d48 into main May 14, 2024
7 checks passed
@samsymons samsymons deleted the sam/remote-messaging-target-percentile branch May 14, 2024 22:53
samsymons added a commit to duckduckgo/macos-browser that referenced this pull request May 15, 2024
samsymons added a commit to duckduckgo/iOS that referenced this pull request May 15, 2024
Task/Issue URL: https://app.asana.com/0/0/1207234800675204/f
Tech Design URL:
CC:

Description:

Client PR for duckduckgo/BrowserServicesKit#809.

This adds support for the targetPercentile feature of RMF.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants