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

refactor(maker): replace zip-folder with cross-zip #325

Merged
merged 1 commit into from
May 3, 2018

Conversation

malept
Copy link
Member

@malept malept commented Sep 18, 2017

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

Replacing zip-folder to get rid of a dependency with a security vulnerability.

ISSUES CLOSED: #322

@malept
Copy link
Member Author

malept commented Sep 18, 2017

We should probably make sure that Mac zip files store symlinks correctly.

@MarshallOfSound
Copy link
Member

@malept It looks like cross-zip uses the same zip args as us but I would like some manual validation that this doesn't break out zipping 👍

@hacdias
Copy link

hacdias commented Nov 27, 2017

Hello, are there any developments on this?

@malept
Copy link
Member Author

malept commented Nov 27, 2017

@hacdias it's blocked on people testing it. See instructions in #331 (comment) for testing.

@malept
Copy link
Member Author

malept commented Mar 24, 2018

To make it a little easier for people to test, I've generated some ZIP files for my demo app:

https://github.com/malept/electron-forge-demo123/releases/tag/v1.4.0

In particular, I would like feedback on the darwin (macOS) zip, to make sure that one still works. I've tested the Linux one.

@malept malept changed the base branch from master to 5.x April 10, 2018 14:46
@MarshallOfSound
Copy link
Member

This should target master methinks, would feel more comfortable now that this maker is extracted into it's own package if this PR just came with simple tests that zips generated with this package can be unzipped using platform specific commands. I.e. Running ps on Windows and unzip on Darwin/*nix

@malept
Copy link
Member Author

malept commented Apr 16, 2018

I'll need some help with the Windows tests then 😄

@MarshallOfSound
Copy link
Member

I'll do windows if you've got dem unix ones 👍

@malept malept changed the base branch from 5.x to master April 16, 2018 01:10
@malept
Copy link
Member Author

malept commented Apr 16, 2018

I'll do windows if you've got dem unix ones 👍

Done!

@MarshallOfSound
Copy link
Member

@malept Can we put the zip / unzip test in the @electron-forge/maker-zip module folder. Just so that tests are scoped right 🤔 core shouldn't fail if zip is broken IMO (I'll be working on relocating some tests over the coming weeks)

@malept
Copy link
Member Author

malept commented Apr 16, 2018

I agree in principle, but the problem is that we'll need to figure out how to persist a packaged app between module tests, otherwise the test run time will skyrocket to unacceptable levels.

@MarshallOfSound MarshallOfSound merged commit e06aa0b into master May 3, 2018
@MarshallOfSound MarshallOfSound deleted the use-cross-zip branch May 3, 2018 04:52
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.

None yet

3 participants