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

Remove unused exports #1612

Merged
merged 3 commits into from
Oct 8, 2024
Merged

Remove unused exports #1612

merged 3 commits into from
Oct 8, 2024

Conversation

arelra
Copy link
Member

@arelra arelra commented Oct 7, 2024

What does this change?

Removes unused exports from the commercial repo.

This is a hangover from when commercial-core and commercial were two separate libraries and we would export from core into commercial. Since merging we can now remove any exported functions which were just for internal sharing and not used by other repositories.

I based the exports that we should keep on this search:
https://github.com/search?q=org%3Aguardian+%40guardian%2Fcommercial&type=code&p=1

Exports required for atoms-rendering were ignored as that is an archived project.

@arelra arelra requested a review from a team as a code owner October 7, 2024 15:42
Copy link

changeset-bot bot commented Oct 7, 2024

🦋 Changeset detected

Latest commit: d88f368

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 Patch

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 Oct 7, 2024

Ad load time test results

For consented, top-above-nav took on average 5127ms to load.
For consentless, top-above-nav took on average 3010ms to load.

Test conditions:

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

Copy link
Contributor

@emma-imber emma-imber left a comment

Choose a reason for hiding this comment

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

🧹

@arelra arelra merged commit 486e77b into main Oct 8, 2024
13 checks passed
@arelra arelra deleted the ravi/remove-unused-exports branch October 8, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants