-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
Product addition: Blizzard Entertainment #84
Conversation
Thanks for submitting this pull request. @milesmcc has been assigned to review these changes, provide feedback, and determine next steps. If you haven't already, please ensure your changes pass all the automated tests. Look in the "Checks" box below and "Files changed" tab to see test results. To learn about the PrivacySpy contribution process, check out the contribution guide.
|
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'll look into it more once I'm back at my flat or have a chance while on campus.
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.
Some more sites that would go under this product
products/blizzard-entertainment.toml
Outdated
"heroesofthestorm.com" | ||
"heroesofthestorm.com", | ||
"overwatchleague.com", | ||
"overwatchcontenders.com" |
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.
Add:
- communitytournaments.blizzardesports.com
- blizzard.gamespress.com
- blizzcon.com
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.
Is it fine if subdomains are included as well?
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 decided not to add the second one, as gamespress.com
is its own domain that may conflict with their policy in the future.
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.
Is it fine if subdomains are included as well?
That would be best asked to @milesmcc or @ibarakaiev, but my assumption is that it's whatever the hostname is + a wildcard (*.google.com would be under Google, but news.google.com could have a seperate entry, for instance)
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.
Checked the extension source and saw that (in theory) the subdomain will be fine. If you want to be certain, I can load up a custom version of the extension and this PR locally and double-check that it doesn't have any issues (you can if you want as well)
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.
Sorry for the long wait for a proper review. Here's my thoughts.
products/blizzard-entertainment.toml
Outdated
@@ -0,0 +1,80 @@ | |||
name = "Blizzard Entertainment" |
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.
Blizzard Entertainment (US)
would be more accurate, as the policies vary between regions.
[rubric.data-deletion] | ||
value = "no" | ||
citations = [ | ||
"You may also submit a verifiable request for us to delete either: (1) your marketing information; or (2) your Battle.net account and your marketing information.", |
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.
This should be automated: The deletion of a Battle.net account is permanent and cannot be reversed; also, all games, assets and history will also be permanently deleted.
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.
Sure, the account deletion is permanent, but they do mention that they may retain some information for reasons like (d) comply with law enforcement requests pursuant to lawful process, (e) for scientific or historical research
, no? This to me suggests that they will still have enough personal information to give away to law enforcement even after the account is deleted.
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.
The first bit is technically unavoidable and something I think many ignore because some information does have to be. Prime example is an order history for tax audits. May be a good idea to get a second opinion from @milesmcc or @ibarakaiev though.
products/blizzard-entertainment.toml
Outdated
|
||
[rubric.history] | ||
value = "no" | ||
notes = ["The policy's history is not made available."] |
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.
Should be last modified. Date at bottom of policy: This Privacy Policy was last updated on October 22, 2020
|
||
[rubric.revision-notify] | ||
value = "no" | ||
citations = ["This Privacy Policy may change from time to time, so please check back periodically to ensure that you are aware of any changes. If we make a material change to this Privacy Policy, we will notify you by posting the change on our websites or in this Privacy Policy and, if necessary, give you additional choices regarding such change before the change becoming effective."] |
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.
Material changes would qualify as revision notifications, and should bring this score up
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 honestly do not know why I put no
here...
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.
Haha all good bruv
@doamatto is right! Incorporate those suggestions and we should be good to go. |
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.
Need to look into these before merging.
Looks pretty solid to me; some things can be changed after #97 is finished, but apart from that it looks basically bang-on to me |
Type of pull request: product addition
Related issues: no issues
A few things worth mentioning.