Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Delete article-aside-adverts as it's no longer used #1733

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

emma-imber
Copy link
Contributor

What does this change?

Deletes article-aside-adverts.ts as it's no longer used. Also adds a comment to the high-merch.ts file to remind us to revisit whether it's still needed after the galleries migration.

Why?

After adding amIUsed to check if these files are still needed, high-merch.ts is still used, but article-aside-adverts.ts is not, so we can delete it.

@emma-imber emma-imber requested a review from a team as a code owner January 9, 2025 16:36
Copy link

changeset-bot bot commented Jan 9, 2025

🦋 Changeset detected

Latest commit: d86939f

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

Copy link
Contributor

github-actions bot commented Jan 9, 2025

Ad load time test results

For consented, top-above-nav took on average 4422ms to load.
For consentless, top-above-nav took on average 2738ms to load.

Test conditions:

  • 5mbps download speed
  • 1.5mbps upload speed
  • 150ms latency

Copy link
Member

@arelra arelra left a comment

Choose a reason for hiding this comment

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

Nice! ✨

@emma-imber emma-imber merged commit 627e730 into main Jan 10, 2025
14 checks passed
@emma-imber emma-imber deleted the ei/delete-article-aside-adverts branch January 10, 2025 09:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants