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

Move im below inline1 #1360

Merged
merged 3 commits into from
May 8, 2024
Merged

Move im below inline1 #1360

merged 3 commits into from
May 8, 2024

Conversation

deedeeh
Copy link
Contributor

@deedeeh deedeeh commented May 2, 2024

What does this change?

This PR changes the order of the im and inline1 slots so now we have the im below inline1.

Occasionally hasInlineMerch will be true but won't serve, due to unsupported targeting being added to the line items, this causes inline1 to be lower than it should be cause it will still try to avoid im.

inline1 is our most valuable slot so changing the order seemed like the best way to avoid this im issue.

This change is tested locally and in CODE with this article using Spacefinder debug which you can use by adding ?sfdebug at the end of the url.

Why?

Reduce revenue impact when the above im issue occurs.

Screenshots

Before After
image image

Copy link

changeset-bot bot commented May 2, 2024

🦋 Changeset detected

Latest commit: cf284fe

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@guardian/commercial Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@deedeeh deedeeh added [beta] @guardian/commercial Add this label to publish an @guardian/commercial beta npm release from a PR and removed [beta] @guardian/commercial Add this label to publish an @guardian/commercial beta npm release from a PR labels May 2, 2024
Copy link
Contributor

github-actions bot commented May 2, 2024

🚀 0.0.0-beta-20240502131530 published to npm as a beta release

@deedeeh deedeeh added the [beta] @guardian/commercial Add this label to publish an @guardian/commercial beta npm release from a PR label May 2, 2024
Copy link
Contributor

github-actions bot commented May 2, 2024

🚀 0.0.0-beta-20240502135404 published to npm as a beta release

@deedeeh deedeeh marked this pull request as ready for review May 2, 2024 14:19
@deedeeh deedeeh requested a review from a team as a code owner May 2, 2024 14:19
@deedeeh deedeeh merged commit 0177571 into main May 8, 2024
12 checks passed
@deedeeh deedeeh deleted the dina/move-im-below-inline1 branch May 8, 2024 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[beta] @guardian/commercial Add this label to publish an @guardian/commercial beta npm release from a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants