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

Removes Specs, gulpfile.cjs and web.config file from makeZipFile #10281

Merged
merged 3 commits into from
Apr 12, 2022

Conversation

sanjeetsuhag
Copy link
Contributor

Fixes #10270

main remove-specs-from-zip
Compressed Size (MB) 66.2 52.3
Uncompressed Size (MB) 192.3 159

I noticed that we don't really do much with the gulpfile.cjs and the web.config files, so this PR removes them too. Not sure if that's the right thing to do, but none of the commands through gulp from inside the zip file anyway.

@cesium-concierge
Copy link

Thanks for the pull request @sanjeetsuhag!

  • ✔️ 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 sanjeetsuhag requested a review from ggetz April 8, 2022 14:26
@ggetz
Copy link
Contributor

ggetz commented Apr 11, 2022

web.config being included was specifically mentioned in CHANGES.md, which leads me to believe it is still included in the zip file for that purpose, even if we ourselves are no longer using IIS to host. If we did chose to remove it, it would be a breaking change.

@ggetz
Copy link
Contributor

ggetz commented Apr 11, 2022

As I understand, the users are not meant to run Cesium dev tasks or build the source code from within the release zip. However, they technically could before this PR.

Without gulpfile.cjs, users cannot run most scripts included in package.json. We do already modify package.json when we create the release zip. I'm curious if it's sensible or good practice to remove the non-dev scripts from the version of package.json included in the zip file.

@sanjeetsuhag
Copy link
Contributor Author

However, they technically could before this PR.

There seems to be something wrong with the default configuration inside the ZIP file. From main, I ran npm run makeZipFile. From inside the contents of the zip file, I tried running npm install, then npm run build and I got the following error:

Screen Shot 2022-04-11 at 10 15 06 AM

@ggetz
Copy link
Contributor

ggetz commented Apr 11, 2022

OK, so if it's not changing the behavior, my second comment shouldn't matter.

@sanjeetsuhag
Copy link
Contributor Author

@ggetz I've restored the web.config file. Is there anything else that needs addressing?

@ggetz
Copy link
Contributor

ggetz commented Apr 12, 2022

Nope, thanks @sanjeetsuhag!

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