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

Marc's final feedback on CUMULUS-748 #422

Merged
merged 2 commits into from
Jul 5, 2018
Merged

Conversation

marchuffnagle
Copy link
Contributor

No description provided.

// skip if the file with the same hash is zipped
// and is a valid zip file
if (fs.existsSync(lambda.local)) {
if (await fs.pathExists(lambda.local)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use an asynchronous operation instead of hanging the process on the fs.existsSync call.

try {
await (util.promisify(yauzl.open))(lambda.local); // Verify yauzl can open the .zip file
return Promise.resolve(lambda);
}
catch (e) {
console.log(`${lambda.local} appears to be an invalid zip file, and will be re-built`);
console.log(`${lambda.local} is invalid and will be rebuilt`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Minor change to the message that's logged.

}
}

let msg = `Zipping ${lambda.local}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved this variable declaration down closer to where it's actually used.

throw (e);
});

try {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this is an async function, we can use a try/catch to make it a little more explicit what is going on.

);
}
);
test.serial('zipLambda: given an invalid zip file generated from a previous run, a new valid lambda file is generated', async (t) => { // eslint-disable-line max-len
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using single quotes for the string, since no string interpolation is being performed. Added // eslint-disable-line max-len to prevent complaints about the long line.

Making this change also changed the indentation of the function, so that's why it looks like there's more changed than there actually is.

@Jkovarik Jkovarik merged commit 3467547 into CUMULUS-748 Jul 5, 2018
@scisco scisco deleted the CUMULUS-748-marc branch July 12, 2018 18:08
laurenfrederick pushed a commit that referenced this pull request Sep 24, 2018
Marc's final feedback on CUMULUS-748
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