Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 WordPress.com themes api into jetpack-mu-wpcom package #41770
base: update/php-8.1
Are you sure you want to change the base?
Move WordPress.com themes api into jetpack-mu-wpcom package #41770
Changes from all commits
280cc48
129dd74
0f5b598
e0853a5
ec17c89
235e0b2
f1fff1b
c473855
48d1113
0f40a7b
d59105a
3f57998
22aac2b
b519aa5
177cb0c
54f45ad
008e630
d56759d
bc27aeb
d0a6f06
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will want these to also run on WPcom in the untangling themes PR, right? Should we move them to run on WPcom now, or only in the untangling themes PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I a bit torn on if we should rename these functions and move the filters over entirely or not. I think it's best to leave it as is. We don't need any of this on WPcom yet. It's probably best to enable it in untangling themes as that's when it's actually needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I agree. If we keep this PR as just a code moving task, then we can consider changes in the future on a case by case basis. If we start changing things as well as moving them its gets harder to reason about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, when I thought about the deploy process for this - because wpcomsh and jetpack-mu-wpcom are deployed at different times, I think it does make more sense to rename the functions now - that way we avoid errors when a function is redefined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was worried about this too, but I think it's not an issue. I believe WoA sites run jetpack-mu-wpcom bundled as wpcomsh. They don't run the WPcom mu-plugin AND wpcomsh. So there should only be one version of anything running, even if deploy cycles are different. We should absolutely confirm to be sure though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I wasn't sure exactly how they fit together but @zinigor suggested it might be a problem and I thought better safe than sorry, since we want to rename anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct! I sometimes get confused with what environment gets affected with what. WoA and Atomic sites should be fine in this regard. The WordPress.com environment though will have these files
removedadded, but not used?If that's so, then I'm sorry for causing additional anxiety about deploying this.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.