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.

Sign up for GitHub

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

Jump to bottom

Pare down build scripts; naming consistancy #10583

Merged
merged 3 commits into from
Jul 22, 2022
Merged

Pare down build scripts; naming consistancy #10583

merged 3 commits into from
Jul 22, 2022

Conversation

ggetz
Copy link
Contributor

@ggetz ggetz commented Jul 21, 2022 •
edited
Loading

Fixes #10313

This PR pares down the number of build command in favor of using flags with the build script as needed. It also makes sure naming conventions are consistant.
@ggetz ggetz requested a review from sanjeetsuhag July 21, 2022 22:12
@cesium-concierge
Copy link

Thanks for the pull request @ggetz!

  • ✔️ Signed CLA found.
  • ❔ CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
  • ❔ Unit tests were not updated.
    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
@sanjeetsuhag
Copy link
Contributor

@ggetz This is a good change. There's a few areas that still need to be updated:

  • The generateDocumentation command is referenced in CesiumSandcastle.js and the Documentation Guide. We should document the build-docs-watch command somewhere.
  • The buildApps command is referenced in the Performance Testing Guide.
  • The makeZipFile command is referenced in the Release Guide in the Wiki.
@ggetz
Copy link
Contributor Author

ggetz commented Jul 22, 2022

Thanks @sanjeetsuhag! I'll update the Release Guide after this PR. I think it may make sense to check this into the repo proper rather than the wiki so we don't have to make separate updates.
sanjeetsuhag
sanjeetsuhag approved these changes Jul 22, 2022
View reviewed changes
Copy link
Contributor

@sanjeetsuhag sanjeetsuhag left a comment

Choose a reason for hiding this comment

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

Thanks @ggetz!

(CI failures are unrelated and random so I'm merging)
@sanjeetsuhag sanjeetsuhag merged commit 47f82fc into main Jul 22, 2022
@sanjeetsuhag sanjeetsuhag deleted the scripts branch July 22, 2022 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants
@ggetz @cesium-concierge @sanjeetsuhag