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

Gutenboarding: Remove page-templates.json #44718

Merged
merged 1 commit into from
Aug 19, 2020

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Aug 5, 2020

Changes proposed in this Pull Request

This file seems unused.

Testing instructions

Make sure this file isn't used anywhere in Calypso, and that Gutenboarding runs properly.

Note that this file was used for a while outside of Calypso, in a REST API endpoint that we use for Gutenboarding. However, I believe that I removed the only part of code where it was evaluated in D41177-code, and @sirreal removed the LOCs that loaded it in D41281-code (Ctr+F page-templates in both diffs).

This means we'll need to exercise some extra caution when landing this (test it properly while it's on staging etc).

@ockham ockham added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Janitorial labels Aug 5, 2020
@ockham ockham requested review from a team August 5, 2020 20:42
@ockham ockham self-assigned this Aug 5, 2020
@matticbot
Copy link
Contributor

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@ockham ockham force-pushed the remove/page-templates-json branch from d451e33 to 9d1fcf3 Compare August 10, 2020 18:03
Copy link
Contributor

@lsl lsl left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for this and the related diffs.

@ockham ockham force-pushed the remove/page-templates-json branch from 9d1fcf3 to 4f57d63 Compare August 11, 2020 16:21
@ockham
Copy link
Contributor Author

ockham commented Aug 12, 2020

e2e tests are failing repeatedly. I'll try once more, but maybe this file is still referenced somewhere 🤔 😕

@ockham ockham force-pushed the remove/page-templates-json branch from 4f57d63 to 6083506 Compare August 13, 2020 10:59
Copy link

@razvanpapadopol razvanpapadopol left a comment

Choose a reason for hiding this comment

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

This file is currently used only in Verticals MC tool as mentioned in #42354 (comment)

@johnHackworth Are we planning to maintain that and maybe update it to use the current design config file?

@ramonjd
Copy link
Member

ramonjd commented Aug 19, 2020

This file is currently used only in Verticals MC tool as mentioned in #42354 (comment)

@razvanpapadopol Not any more 😄 D48129-code

This means we'll need to exercise some extra caution when landing this (test it properly while it's on staging etc).

@ockham Thanks for looking at this.

We were tracking the "clean up" over here: https://github.com/orgs/Automattic/projects/147#card-43574014

I created a PR that goes further than this, removing all the JSON files we don't use (and probably never have used) in production

#44932

I can rebase if you decide to go ahead with this one (LGTM by the way), or feel free to duplicate and close mine. However you see fit.

🙏

@razvanpapadopol razvanpapadopol removed DO NOT MERGE [Status] Blocked / Hold [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Aug 19, 2020
@razvanpapadopol razvanpapadopol merged commit d4c4cc4 into master Aug 19, 2020
@razvanpapadopol razvanpapadopol deleted the remove/page-templates-json branch August 19, 2020 04:29
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.

5 participants