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 rules for Pubtech (CMP) #25

Merged
merged 14 commits into from
Sep 16, 2022
Merged

Add rules for Pubtech (CMP) #25

merged 14 commits into from
Sep 16, 2022

Conversation

icodebyamanda
Copy link
Collaborator

@icodebyamanda icodebyamanda requested a review from muodov August 21, 2022 19:51
Copy link
Member

@muodov muodov left a comment

Choose a reason for hiding this comment

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

Nice, just needs some polishing :)
you can also find more test examples on https://publicwww.com/websites/%22pubtech-cmp%22/

tests/pubtech.spec.ts Outdated Show resolved Hide resolved
rules/autoconsent/pubtech.json Outdated Show resolved Hide resolved
rules/autoconsent/pubtech.json Outdated Show resolved Hide resolved
rules/autoconsent/pubtech.json Outdated Show resolved Hide resolved
@icodebyamanda
Copy link
Collaborator Author

@muodov it's good to be reviewed.
Passed: Self-test, local test & jenkins job

rules/autoconsent/microsoft.json Outdated Show resolved Hide resolved
rules/autoconsent/pubtech.json Outdated Show resolved Hide resolved
rules/autoconsent/pubtech.json Outdated Show resolved Hide resolved
rules/autoconsent/pubtech.json Outdated Show resolved Hide resolved
@icodebyamanda
Copy link
Collaborator Author

icodebyamanda commented Sep 5, 2022

@muodov I made some corrections and left a comment about the potential risk for the buttons to switch order and asked if we should park this project under the autogenerated selectors websites. Comment here: #25 (comment)

Also not sure I understand your comment on the "OR" and parenthesis.

Otherwise self, local and jenkins tests are all passing.

Copy link
Member

@muodov muodov left a comment

Choose a reason for hiding this comment

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

looking better! Just a couple of changes and should be good to go

@icodebyamanda
Copy link
Collaborator Author

@muodov made the changes requested (conditional rule for OptIn and the eval logic). Some whitespace in the optIn rule needed to becommitted which explains the third commit. Passed: Self-test, local test & jenkins job.

Copy link
Member

@muodov muodov left a comment

Choose a reason for hiding this comment

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

LGTM! 🙏

@muodov muodov merged commit fa83a98 into main Sep 16, 2022
@muodov muodov deleted the pubtech-CMP branch September 16, 2022 11:44
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