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

Bug(cli): code.tar.gz file remains after error occurs #99

Closed
wants to merge 2 commits into from

Conversation

Vaibhav91one
Copy link

@Vaibhav91one Vaibhav91one commented Oct 9, 2023

What does this PR do?

  • This PR mainly implements a Try catch and finally block to the functionCreateDeployment.
  • As if anything happens in between the finally block will always execute and will remove the remaining file.

Test Plan

N/A

Related PRs and Issues

Have you read the Contributing Guidelines on issues?

Yes

Copy link

@gewenyu99 gewenyu99 left a comment

Choose a reason for hiding this comment

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

Hey, could you please clean up the commit and remove all the unnecessary white space type changes so we can find what changed more easily?

@Vaibhav91one
Copy link
Author

Hey, could you please clean up the commit and remove all the unnecessary white space type changes so we can find what changed more easily?

Sure.

@Haimantika Haimantika requested a review from gewenyu99 October 31, 2023 12:26
@stnguyen90 stnguyen90 self-requested a review October 31, 2023 22:24
@@ -405,6 +405,7 @@ const functionsCreateDeployment = async ({ functionId, code, activate, entrypoin
/* @param {string} entrypoint */
/* @param {string} commands */

try{
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Let's not have such a big try/catch
  2. This repo is read-only and the change should be done with the sdk-generator repo.

@stnguyen90
Copy link
Contributor

I'm going to add the hacktoberfest-accepted label just so you get credit, but we'll probably close this PR since we need the change in the SDK generator repo.

@gewenyu99
Copy link

Hey there! There were a lot of big PRs during this Hacktoberfest, and we wanted to give everyone ample time to collaborate with our engineering team. If you were able to merge your PRs during October, amazing. If it’s still not merged, don’t worry about it either. Either way, we’ve got your Hacktoberfest swag minted and ready to ship.

Please comment with your Discord username here so we can contact you about your shipping information to deliver your Hacktoberfest swag.

@stnguyen90 stnguyen90 closed this May 20, 2024
@stnguyen90
Copy link
Contributor

closing as change needs to be done in sdk-generator repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants