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

fix(docs-infra): add cookie consent gtag event default state #54574

Closed
wants to merge 1 commit into from

Conversation

bencodezen
Copy link
Contributor

@bencodezen bencodezen commented Feb 22, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.dev application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

No Google Analytics consent code existed.

Issue Number: N/A

What is the new behavior?

Google Analytics code for cookie consent is now added per the docs.

The other half of this PR that grants cookie consent can be found at angular/dev-infra#1805

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release area: docs-infra Angular.dev application and infrastructure labels Feb 23, 2024
@ngbot ngbot bot added this to the Backlog milestone Feb 23, 2024
@AndrewKushnir
Copy link
Contributor

@bencodezen could you please also change the commit message to start with fix(docs-infra): ..., so that we can merge the change into the patch branch (feature commits can only go to the minor/major branches)?

@bencodezen
Copy link
Contributor Author

@bencodezen could you please also change the commit message to start with fix(docs-infra): ..., so that we can merge the change into the patch branch (feature commits can only go to the minor/major branches)?

@AndrewKushnir Absolutely! Thanks for calling that out. I've updated the PR with the correct doc comment and message!

@angular-robot angular-robot bot removed the detected: feature PR contains a feature commit label Feb 23, 2024
@AndrewKushnir AndrewKushnir changed the title fix: add default cookie consent denied state for gtag fix(docs-infra): add cookie consent gtag event default state Feb 24, 2024
@AndrewKushnir AndrewKushnir removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Mar 5, 2024
@AndrewKushnir
Copy link
Contributor

@bencodezen LGTM, thanks for the updates. I've removed the "review" label, please add the "merge" label when the PR is ready for merge. Thank you.

@bencodezen bencodezen added the action: merge The PR is ready for merge by the caretaker label Mar 6, 2024
@bencodezen bencodezen added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: merge The PR is ready for merge by the caretaker labels Mar 7, 2024
@AndrewKushnir AndrewKushnir removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Mar 8, 2024
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM for dev-infra (been waiting since this is blocked, but not ready for review— but want to get it off my list)

@bencodezen bencodezen force-pushed the default-cookie-banner-state branch 4 times, most recently from ffb3422 to ec6d328 Compare March 27, 2024 16:56
@bencodezen bencodezen force-pushed the default-cookie-banner-state branch 2 times, most recently from 3154148 to 3226213 Compare April 23, 2024 15:11
@bencodezen bencodezen added target: minor This PR is targeted for the next minor release action: merge The PR is ready for merge by the caretaker and removed state: blocked target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker labels Apr 23, 2024
This PR sets a default state for cookie consent of 'denied'. The other part of the PR exists in the angular/dev-infra repo which will grant permission when the user accepts the cookie banner.
@bencodezen bencodezen added action: merge The PR is ready for merge by the caretaker action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: merge The PR is ready for merge by the caretaker labels Apr 23, 2024
@bencodezen
Copy link
Contributor Author

Finally managed to get everything passing in the CI! 🥳 Ready for review and then merge!

@bencodezen bencodezen added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 24, 2024
@thePunderWoman thePunderWoman removed the request for review from devversion April 24, 2024 18:23
@AndrewKushnir AndrewKushnir added target: rc This PR is targeted for the next release-candidate and removed target: minor This PR is targeted for the next minor release labels Apr 25, 2024
@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit 0b53fdb.

AndrewKushnir pushed a commit that referenced this pull request Apr 25, 2024
This PR sets a default state for cookie consent of 'denied'. The other part of the PR exists in the angular/dev-infra repo which will grant permission when the user accepts the cookie banner.

PR Close #54574
@bencodezen bencodezen deleted the default-cookie-banner-state branch April 25, 2024 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker area: docs-infra Angular.dev application and infrastructure target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants