Skip to content

Better implementation for loading files from a ZIP#75

Closed
bertfrees wants to merge 2 commits intomasterfrom
fix-load-from-zip
Closed

Better implementation for loading files from a ZIP#75
bertfrees wants to merge 2 commits intomasterfrom
fix-load-from-zip

Conversation

@bertfrees
Copy link
Member

because cx:unzip can not be used to load as html or text.

See also commit 4ae7bb4.

because cx:unzip can not be used to load as html or text.

See also commit 4ae7bb4.
@bertfrees bertfrees requested a review from josteinaj September 23, 2019 16:13
@bertfrees
Copy link
Member Author

@josteinaj Can you please review this code?

@josteinaj
Copy link
Member

Not tested locally but looks good 👍

@bertfrees
Copy link
Member Author

@josteinaj I would like to see this in a new release. Can you do it?

By the way what are we going to do with my other pull requests?

@josteinaj
Copy link
Member

Sorry, I've had too much to do, and I've forgotten about these PRs. I'll try to find time to look at it soon.

I haven't reviewed the PRs but I'm sure we can rebase or merge them all into the release.

@bertfrees
Copy link
Member Author

Cool, thanks.

The other PR's are not urgent, I just want to know what to do with them. If you don't like them we can drop them. Or if you think they need more work...

Copy link
Member

@josteinaj josteinaj left a comment

Choose a reason for hiding this comment

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

Seems fine. I don't think the tests cover the case with zipped files, but I suspect you've tested it on other projects, and the normal tests passes, so this should be fine.

@josteinaj josteinaj mentioned this pull request Aug 25, 2020
@josteinaj
Copy link
Member

Fixed in #76 (v1.4.0)

@josteinaj josteinaj closed this Aug 25, 2020
@bertfrees
Copy link
Member Author

Ah yes a test would have been a good idea indeed. I've indeed tested with other projects.

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.

2 participants