Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Stop interactive create or update on validation errors #1382

Merged
merged 1 commit into from
Jan 13, 2020

Conversation

spalladino
Copy link
Contributor

@spalladino spalladino commented Jan 13, 2020

The force flag was being passed to push from both create and upgrade to force pushing the implementation contracts. This caused the contracts to be pushed even if they had validation errors. This commit removes that flag so the create or upgrade fails if there are any errors.

Fixes #1348

@spalladino spalladino changed the title WIP Stop interactive create or update on validation errors Jan 13, 2020
@spalladino spalladino force-pushed the fix/abort-deploy-on-validation-errors-#1348 branch from 7e2a139 to 5d1937f Compare January 13, 2020 18:44
@spalladino spalladino requested a review from frangio January 13, 2020 18:49
@spalladino spalladino marked this pull request as ready for review January 13, 2020 18:49
Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

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

Yay less lines of code!

}
}
const { interactive } = options;
if (!interactive) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

I personally prefer 1) avoiding early returns, and 2) always using braces. The latter should be a linter rule if we even agree to it, so I won't insist on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I like returns either at the beginning or the end, though not in the middle. Returns at the very first lines of the function feel natural as they are kind of "validations" of whether we want to run or not. Agree on avoiding them in the middle.

  2. I particularly line single-line ifs when it's a return. Seems like we're on opposite sides on this one!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, just changed this so the return is on the very first line.

The force flag was being passed to push from both create and upgrade to force pushing the implementation contracts. This caused the contracts to be pushed even if they had validation errors. This commit removes that flag so the create or upgrade fails if there are any errors.

Fixes #1348
@spalladino spalladino force-pushed the fix/abort-deploy-on-validation-errors-#1348 branch from 5d1937f to d517f42 Compare January 13, 2020 22:40
@spalladino spalladino added the status:ready-to-merge Order mergify to merge label Jan 13, 2020
@mergify mergify bot merged commit 1025c22 into master Jan 13, 2020
@mergify mergify bot deleted the fix/abort-deploy-on-validation-errors-#1348 branch January 13, 2020 22:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:ready-to-merge Order mergify to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Command create bypasses contract validations
2 participants