-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[WP 6.2] Rename experiments package to private-apis #47840
Conversation
@Mamaduka @ntsekouras @gziolo Any idea why this fails? The failures say:
The PR targeted for Trunk works (except for mobile tests): #47839 Is the CI pipeline for the 6.2 branch special in any way? |
@adamziel, CI pipelines should be the same. Are there any prerequisites for the change? Those will need to be backported. |
@Mamaduka thanks! There should be no pre-requisites, this PR only renames the |
I get the same error when running |
All PRs should target the regular trunk branch and with the "backport label", we have workflow in places to backport this when doing the package releases... (Unless there's a very specific reason for this PR that I'm missing) |
Oh I just saw that there's already a PR that targets trunk. I'm not understanding why we need two PRs then? What am I missing? |
I think the plan in trunk is to add the gutenberg conditional mentioned in #47785 (comment) My suggestion was to limit 6.2 to renaming, and add the Gutenberg conditional in trunk. Fewer changes to the published packages during beta, etc, etc. If you're happy to the rename in one PR for trunk, and add the gutenberg conditionals in another then that works too. I'm not sure how possible that is though as I rarely publish NPM packages. |
Understood, what ever make sense for you. Personally I'd have done the addition of conditions in its own dedicated PR. |
I also believe we should have on PR on trunk only to rename. And handle the other changes in different PR. |
Something went wrong in this PR: I see a few README files with content removed. I'm afraid that it won't work without additional changes if you move In a few places, we assume that all WordPress packages are located directly in the packages folder, not its subfolders. |
@youknowriad I prepared a dedicated PR mostly to save @ntsekouras and @Mamaduka the effort of resolving merge conflicts – I tried cherry-picking the branch targeting
@gziolo yup, I'm looking into this now. |
Size Change: -861 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
Flaky tests detected in 7897c03. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4124569028
|
@ntsekouras @Mamaduka this is ready for merging 🎉 There's also a trunk-based version – feel free to choose whether you'd prefer to merge both PRs separately, or close this one and cherry-pick the other one. |
This PR doesn't add the conditions @youknowriad. It only renames the package. Actually, I wasn't planning on spinning a PR to add the conditions – they're already supported and don't need any additional pull requests. Now it's up to API authors to use them. I will only start a Pull Request to update the coding guidelines accordingly. |
I'm really happy either way. To be 100% honest, I had a really dumb moment when I first asked for separate PRs and didn't even think of doing the rename and exports in trunk across two tickets. @adamziel Sorry if that dumb moment ends up wasting your time. |
Let's close this one and back port #47839 instead :) |
What?
This PR targets the wp/6.2 branch
Renames the @wordpress/experiments package to @wordpress/private-apis to avoid confusion between internal APIs and experimental APIs. See #47785
Testing Instructions
Confirm the checks pass
cc @youknowriad @mcsf @peterwilsoncc @ntsekouras