-
Notifications
You must be signed in to change notification settings - Fork 26
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
Bug 1689547 - First attempt at providing a ping encryption plugin #96
Conversation
8ebe144
to
675bfbe
Compare
@acmiyaguchi I attended to your review comments in 9b69d77 this commit :) |
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.
Thanks for addressing my comments :) What do you still have left in this PR?
If I have your green light on the encryption I'll just request Alessio's review for sanity check on the Glean.js specific parts and merge! Thanks @acmiyaguchi :D |
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.
I left a few comments below. Did you check that "log pings" allow us to ... log pings with this?
Nope, but I can check that and even go further and add a test for it. |
Yes please :) |
Co-authored-by: Anthony Miyaguchi <acmiyaguchi@gmail.com>
6dbf549
to
4a3cd1b
Compare
Opening this as a draft to get feedback on the encryption implementation.