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

Delete old commercial code to fix dev build #27771

Merged
merged 1 commit into from
Feb 12, 2025
Merged

Conversation

DanielCliftonGuardian
Copy link
Contributor

@DanielCliftonGuardian DanielCliftonGuardian commented Feb 11, 2025

What is the value of this and can you measure success?

Removes CommercialLifecycle route as in #27755 and the test pages route as in here #27726 for dev-build app as has been done in the commercial app

Presumably we could delete this file now also https://github.com/guardian/frontend/blob/09d382fd6531117c0c5910e5cf5267a54d707471/commercial/app/controllers/CreativeTestPage.scala

What does this change?

Fix dev-build

@DanielCliftonGuardian DanielCliftonGuardian requested a review from a team as a code owner February 11, 2025 14:55
@arelra arelra requested a review from a team February 11, 2025 15:57
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.

I think we can delete CreativeTestPage.scala too.

It seems to be behind the 'merchandising components' link on the admin page which now errors:

https://m.code.dev-theguardian.com/commercial/test-page

I believe this page was related to the travel and jobs features which have been removed.

We can also delete it from the menu:

<li><a href="@Configuration.commercial.testDomain/commercial/test-page">Merchandising components</a></li>

@Jakeii do you agree?

@Jakeii
Copy link
Member

Jakeii commented Feb 11, 2025

I think we can delete CreativeTestPage.scala too.

It seems to be behind the 'merchandising components' link on the admin page which now errors:

m.code.dev-theguardian.com/commercial/test-page

I believe this page was related to the travel and jobs features which have been removed.

We can also delete it from the menu:

<li><a href="@Configuration.commercial.testDomain/commercial/test-page">Merchandising components</a></li>

@Jakeii do you agree?

Yep sounds good!

@arelra
Copy link
Member

arelra commented Feb 12, 2025

@DanielCliftonGuardian feel free to merge this so dev-build is fixed. I'll pick up the other deletions.

@prout-bot
Copy link
Collaborator

Seen on ADMIN-PROD (merged by @DanielCliftonGuardian 12 minutes and 13 seconds ago)

@prout-bot
Copy link
Collaborator

Seen on FRONTS-PROD (merged by @DanielCliftonGuardian 13 minutes and 20 seconds ago)

@DanielCliftonGuardian
Copy link
Contributor Author

@DanielCliftonGuardian feel free to merge this so dev-build is fixed. I'll pick up the other deletions.

Thanks @arelra I created a follow up PR with these changes here #27778

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.

4 participants