-
Notifications
You must be signed in to change notification settings - Fork 846
Add plugin jetpack-mu-wpcom-plugin #28479
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
Conversation
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped. Jetpack Mu wpcom plugin plugin:
Mu Wpcom plugin plugin:
|
|
@anomiex Should this have a mirror repo? Also, can we just modify the current "jetpack-mu-wpcom" entry in the |
I think the tooling will require that. Without a mirror repo it won't make an entry in the "jetpack-build" artifact, so the rest of the stuff won't have anything to run on.
Yes, that should work. |
|
@anomiex Could you help me understand the failing |
|
Ah, the Post-Build version is trying to read the composer.json from trunk rather than this branch, and it doesn't exist in trunk... Let's see if 51a1671 fixes it. |
|
Ick, looks like the changes here basically need #28510. I suppose we could apply a similar change to the copy of the job in build.yml, which will make this and that conflict... |
|
Random complaint: Can we name the directory something that doesn't start with "jetpack"? I'm used to typing |
Done! So it sounds like #28510 needs to merge first. And should I wait to update the beta manifest until after this merges? |
|
I'd probably update the manifest just before merging this just so I could watch for the resulting trunk build being picked up correctly. |
|
D'oh, the changes to the workflow need to be done in a separate PR because it runs off of trunk. I'll create that momentarily. |
|
@anomiex Looks like the workflow is working now 👍 Anything else on this one? I can update the beta manifest once this is ready to merge. |
anomiex
left a comment
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.
Let's go for it. Note you'll also have to update the jetpack-downloader script, and I think @sdixon194's shiny button script too if that has been merged already.
Proposed changes:
This plugin will:
jetpack-mu-wpcomvia the Jetpack Beta Tester pluginOther information:
Jetpack product discussion
Internal: p1674159922879619-slack-C01U2KGS2PQ
Does this pull request change what data or activity we track or use?
No.
Testing instructions:
Proofread and make sure it all builds.