-
-
Notifications
You must be signed in to change notification settings - Fork 716
[FP][IMP] website_cookiefirst: Replace deprecated Cookiefirst functionality #1113
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
base: 18.0
Are you sure you want to change the base?
[FP][IMP] website_cookiefirst: Replace deprecated Cookiefirst functionality #1113
Conversation
NICO-SOLUTIONS
commented
Jul 1, 2025
- replace banner.js with new consent.js script using domain and identifier
- forward ported from V17 PR ([IMP] website_cookiefirst: Replace deprecated Cookiefirst functionality #1110)
@eugenios73 |
@ioans73 added the fix in this existing fowardport. |
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.
LGTM
This PR has the |
@OCA/website-maintainers |
You have to squash commits and use proper commit message: |
And next time, it's better that you do only a pull request for one version, and when merged, then do the fw-ports, as if not, any change have to be propagated to each of the PRs, giving you more work, and adding noise to the email subscribers. |
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.
technical review is ok
@pedrobaeza |
Yes, in this version 18, there's no need to keep both things as separated, as I understand that both belongs to the same concept: replace deprecated functionality. In 17, as one patch was already merged, there's no other solution than to add an extra commit, but here, we can keep the commit history clean. |
f60f8bd
to
0ea9a86
Compare
0ea9a86
to
f746c2d
Compare