-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Sandcastle full release #12937
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
Sandcastle full release #12937
Conversation
Thank you for the pull request, @jjspace! ✅ We can confirm we have a CLA on file for you. |
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.
Updates look good! I'm about to test locally before we go ahead and merge.
In the meantime, could you also add an update to CHANGES.md
with a short summary of the updates here? It can be outside of the package updates like the message from the last release.
<Anchor | ||
href="https://cesium.com/downloads/cesiumjs/releases/1.134/Apps/Sandcastle/index.html" | ||
tone="accent" | ||
> |
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.
Would you mind opening an standalone issue for the plan to fully remove the old Sandcastle? I think it'll be helpful to discuss apart from the "new" Sandcastle plans themselves.
</li> | ||
<li> | ||
<a href="Apps/Sandcastle/index.html">Sandcastle</a> | ||
<a href="Apps/Sandcastle2/index.html">Sandcastle</a> |
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.
Should build-sandcastle
become part of the build step? Either the build itself, or as part of the initial run on npm start
?
It was a bit confusing when I ran:
npm install
npm run make-zip
npm start
but when I clicked on Sandcastle, I still saw

This fix could be a fast follow PR, as it does not impact the release itself.
For the sake of time, I went ahead and updated Please make sure to review the two comments above and determine if any follow-ups are needed. |
Description
prod
github workflow to build the new sandcastle and deploy it to the bucket forsandcastle.cesium.com
Issue number and link
Testing plan
Mimic prod build
Mimic the release zip process
npm install
andnpm start
work in that directoryAuthor checklist
CONTRIBUTORS.md
CHANGES.md
with a short summary of my change