feat: Add zip method #68
Conversation
Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com>
| console.error( | ||
| `Zip file creation failed for destination: "${dest}", source: "${source}". Error: ${error}.`, | ||
| ); |
There was a problem hiding this comment.
One thing I'm hung up on is whether or not its better to let whoever is invoking the function to print the error. Printing to console is a user facing behaviour and having a low level utility function do that seems more surprising than just throwing the error.
There was a problem hiding this comment.
Oops, added this comment, but forgot to submit the review. Fine to disregard, but still wanted to bring it up.
There was a problem hiding this comment.
I agree that an error thrown by a low-level function may be unexpected. However, given that the zipping behavior is determined by user settings in the sauce configuration, such an error could be understandable and helpful.
It allows users to debug and modify their configuration as necessary. This is different from generating JUnit or Sauce reports, where a failure could be misinterpreted as a test failure, as those operations are not directly controlled by the user.
There was a problem hiding this comment.
@tianfeng92 Mike is talking about removing this log line and instead throwing the error all the way back to the caller (the runner in that case). The caller (runner) is then the one to handle (i.e. log) the error.
There was a problem hiding this comment.
Oops, added this comment, but forgot to submit the review. Fine to disregard, but still wanted to bring it up.
Now that you bring this up, I'd personally prefer that approach. Generally I'm in favor of keeping user facing messaging out of low level libs. Having user facing messages in low level libs, prevents the lib from being used more broadly and robs the caller of controlling what the user should see.
There was a problem hiding this comment.
Make sense. Let me draft a PR to address it.
One-line summary
Add zip method: this method receives source directory and destination file and zips file accordingly.