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

[MIG][17.0] website_cookiebot #1044

Merged
merged 14 commits into from
Aug 22, 2024
Merged

Conversation

adriresu
Copy link

Standard migration

antonio-trey and others added 4 commits June 18, 2024 08:49
- Reorganize templates.
- Make templates noupdate=0, because if user edits them, it will be in the COW'd, website-specific version, so no problem on that.
- Rename `cookiebot_id` to `cookiebot_dgid` to avoid confusion with X2many fields.
- Remove ICP support, as the DGID should be per website.
- Add ID placeholders to Cookie Declaration page editable sections.
- Ensure Google Analytic script is blocked before consenting statistics. FTR it wasn't actually being blocked.
- Improve instructions.
- Split README.
- Apply pre-commit.

@Tecnativa TT32828
@adriresu adriresu force-pushed the 17.0-mig-website_cookiebot branch 2 times, most recently from 76cd3a5 to 20e946b Compare June 20, 2024 08:05
@adriresu
Copy link
Author

Please @dalonsod, can you check It?

@pedrobaeza
Copy link
Member

/ocabot migration website_cookiebot

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Jun 20, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Jun 20, 2024
12 tasks
@dalonsod
Copy link

There's still a line for res.config.settings not covered by tests, could you check and add pending test for cookiebot_enabled field change?

image

@adriresu
Copy link
Author

There's still a line for res.config.settings not covered by tests, could you check and add pending test for cookiebot_enabled field change?

image

Test added and ready to check.

Copy link

@dalonsod dalonsod left a comment

Choose a reason for hiding this comment

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

@adriresu
Copy link
Author

👍 Please clean commit history as described at https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate

This is my first time merging commits in a OCA repository, but I believe I have merged the corresponding commits correctly. However, some of the commits cause issues when their order is changed for merging as It is mencioned in the documentation.
Hope It is ok.

@chienandalu
Copy link
Member

Hi, could you include #1055

@Anxo82 Anxo82 force-pushed the 17.0-mig-website_cookiebot branch 2 times, most recently from 52bb6a3 to 495cf9b Compare August 21, 2024 10:28
If we don't remove the key the assets will keep trying to load, which
isn't what we'd expect.

TT50520
@Anxo82
Copy link

Anxo82 commented Aug 21, 2024

Done! Review please @chienandalu @dalonsod
Thanks

Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-1044-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit e1d1305 into OCA:17.0 Aug 22, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 531aeed. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.