Skip to content

Conversation

@chrisgervang
Copy link
Collaborator

For #309

Split out from #285

@chrisgervang chrisgervang requested a review from Copilot March 19, 2025 00:02
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the encoder modules to remove the legacy 'archive' configuration and standardize ZIP as the only supported archive format, updating both code and documentation.

  • Removed the archive option from PNG and JPEG encoders.
  • Updated examples and documentation to reflect ZIP encryption.
  • Deleted TAR-related utility and builder files.

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

Show a summary per file
File Description
modules/core/src/encoders/frame-encoder.ts Removed archive configuration from PNG and JPEG format settings.
examples/website/basic-basemap/app.jsx Removed archive property from formatConfigs and simplified component structure.
examples/website/basic-basemap-mapbox-legacy/app.jsx Removed archive property and adjusted formatting in usage of DeckGLOverlay.
examples/website/basic-basemap-mapbox/app.jsx Removed archive property and adjusted formatting in usage of DeckGLOverlay.
modules/react/src/components/quick-animation.tsx Updated formatConfigs to remove the archive field in PNG settings.
examples/website/camera/app.jsx Removed archive from PNG settings and updated JPEG configuration in formatConfigs.
modules/react/src/components/export-video/export-video-panel-container.tsx Removed archive property from PNG and JPEG format configurations.
docs/api-reference/encoder/jpeg-sequence-encoder.md Updated encoder description and configuration to reflect ZIP usage instead of TAR.
examples/website/terrain/app.jsx Removed archive property from PNG settings.
examples/website/trips/app.tsx Removed archive property from PNG settings.
docs/api-reference/encoder/png-sequence-encoder.md Updated encoder description and configuration to reflect ZIP usage instead of TAR.
modules/core/src/encoders/video/jpeg-sequence-encoder.ts Removed TAR support; standardized JPEG sequence encoding to use ZIP exclusively.
modules/core/src/encoders/video/png-sequence-encoder.ts Removed TAR support; standardized PNG sequence encoding to use ZIP exclusively.
modules/core/src/encoders/tar/* Deleted unused TAR-related modules (header, tar-builder, tar, utils).
Comments suppressed due to low confidence (1)

modules/core/src/encoders/video/jpeg-sequence-encoder.ts:24

  • [nitpick] Consider standardizing the access of ZipWriter.extensions across encoders. Since PNGSequenceEncoder does not use optional chaining, removing it here could improve consistency.
this.extension = `.${ZipWriter.extensions?.[0]}`;

@coveralls
Copy link

coveralls commented Mar 19, 2025

Pull Request Test Coverage Report for Build 13961936087

Details

  • 5 of 28 (17.86%) changed or added relevant lines in 5 files are covered.
  • 6 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.6%) to 49.452%

Changes Missing Coverage Covered Lines Changed/Added Lines %
modules/react/src/components/export-video/export-video-panel-container.tsx 0 1 0.0%
modules/core/src/encoders/video/jpeg-sequence-encoder.ts 0 7 0.0%
modules/core/src/encoders/video/png-sequence-encoder.ts 2 9 22.22%
modules/react/src/components/quick-animation.tsx 2 10 20.0%
Files with Coverage Reduction New Missed Lines %
modules/core/src/encoders/video/jpeg-sequence-encoder.ts 3 50.0%
modules/core/src/encoders/video/png-sequence-encoder.ts 3 55.1%
Totals Coverage Status
Change from base Build 13935751863: 0.6%
Covered Lines: 2446
Relevant Lines: 5004

💛 - Coveralls

@chrisgervang
Copy link
Collaborator Author

Tested website examples 👍

@chrisgervang chrisgervang merged commit 9631f51 into master Mar 20, 2025
1 check passed
@chrisgervang chrisgervang deleted the chr/remove-tar branch March 20, 2025 04:32
@chrisgervang chrisgervang mentioned this pull request Mar 20, 2025
8 tasks
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.

4 participants