-
Notifications
You must be signed in to change notification settings - Fork 31
chore(marketplace): use hardhat ignition #1195
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
Conversation
1f2caf1
to
bb505db
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick suggestion...
0f0e8b1
to
730df2e
Compare
tests/integration/hardhatprocess.nim
Outdated
assert execResult.status == 0 | ||
except CancelledError as e: | ||
raise e | ||
except Exception as e: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except Exception as e: | |
except CatchableError as e: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used Exception
because the compiler was complaining Error: workingDir(node) can raise an unlisted exception: Exception
730df2e
to
6805b54
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! 🙏
Seems like the tests are failing on the test that you reduced the polling for? Maybe try to revert it? |
Yeah actually I would prefer to have #1268 merged first and rebase this PR on master. |
a8c21bd
to
749f27a
Compare
749f27a
to
0499e89
Compare
0499e89
to
4966e40
Compare
This PR update the contracts module in order to use hardhat ignition for deployments and replace the deleted increaseAllowance by allowance / approve functions.