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

fix[sdkCore][Symbols]: ENG-7640 Set preview to true only if its not a symbol call #3805

Conversation

anaghav2023
Copy link
Contributor

Description

If we are using Symbol in any page. It reflects the unpublished changes. This is happening only on the content editor and not the actual website.

Before/After loom
https://www.loom.com/share/4ac7c396c70a4887a19b9f0eec295c63

@anaghav2023 anaghav2023 requested a review from a team as a code owner December 30, 2024 06:39
@anaghav2023 anaghav2023 requested review from sidmohanty11 and removed request for a team December 30, 2024 06:39
Copy link

changeset-bot bot commented Dec 30, 2024

🦋 Changeset detected

Latest commit: 79cccbb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@builder.io/sdk Patch
@builder.io/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

nx-cloud bot commented Dec 30, 2024

View your CI Pipeline Execution ↗ for commit 79cccbb.

Command Status Duration Result
nx test @e2e/sveltekit ❌ Failed 4m 50s View ↗
nx test @e2e/react-sdk-next-pages ❌ Failed 1m 56s View ↗
nx test @e2e/qwik-city ✅ Succeeded 9m 15s View ↗
nx test @e2e/nextjs-sdk-next-app ✅ Succeeded 8m 17s View ↗
nx test @e2e/nuxt ✅ Succeeded 7m 59s View ↗
nx test @e2e/angular ✅ Succeeded 6m 37s View ↗
nx test @e2e/solid-start ✅ Succeeded 6m 19s View ↗
nx test @e2e/react-native ✅ Succeeded 6m 2s View ↗
Additional runs (34) ✅ Succeeded ... View ↗

☁️ Nx Cloud last updated this comment at 2025-01-02 09:17:34 UTC

Copy link

@builder-adi builder-adi left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@sidmohanty11 sidmohanty11 left a comment

Choose a reason for hiding this comment

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

LGTM! @anaghav2023

Last thing: it needs changeset. For that you can run yarn g:changeset and add changesets (patch update) for both gen1 React (@builder.io/react) and gen1 core (@builder.io/sdk) SDKs

@anaghav2023
Copy link
Contributor Author

LGTM! @anaghav2023

Last thing: it needs changeset. For that you can run yarn g:changeset and add changesets (patch update) for both gen1 React (@builder.io/react) and gen1 core (@builder.io/sdk) SDKs

@sidmohanty11 I have made change only in the core still I need to add changesets for react as well? Then do I need to add it for other frameworks as well?

@sidmohanty11
Copy link
Contributor

@anaghav2023 as @builder.io/sdk (core) is a dependency of @builder.io/react (gen1 react), the bot will trigger a version bump of the gen1 react package as well, using the latest version. So it's better if we add the changeset for it so that it reflects what came in - in the latest release (which is just the version bump). Otherwise, it will bump the version, but no changelog will be added. We don't need to add for other SDKs (@builder.io/sdk-*) as they are Gen2 SDKs, and they don't have the gen1 core SDK as their dependency; they are solely present in the packages/sdks/* directory.

Copy link
Contributor

@sidmohanty11 sidmohanty11 left a comment

Choose a reason for hiding this comment

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

💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants