Skip to content

Conversation

@JJ
Copy link
Contributor

@JJ JJ commented Oct 17, 2022

✍️ Description

This code was the only one using the construction zipFile.getContent, which apparently disappeared a some point in time. It's been refactored to work as intended, getting the content of a single ZipEntry. Adds a tests which fails if the original content was used.

🏗️ Fixes PROD4POD-1940

This was found while doing #1219, so it's a blocker for that one.

ℹ️ Other information

Deep tests have not really been performed; they're mainly sanity tests. Rendering tests are not checked either. Better mocks would probably be needed for better tests.

(Unrelated) coupling with zipFile has been eliminated from the function, since it's no longer needed. The function was returning a data structure which included it, not doing that does not seem to affect the outcome.

♥️ Thank you!

@JJ JJ requested review from RichardSto and chisandrei October 17, 2022 10:24
Copy link
Contributor

@fhd fhd left a comment

Choose a reason for hiding this comment

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

LGTM

@fhd fhd merged commit 643f174 into main Nov 3, 2022
@fhd fhd deleted the ♻-zipfile-getContent branch November 3, 2022 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants