Skip to content
This repository was archived by the owner on Jul 5, 2022. It is now read-only.

The fixes to issue #19 #20

Open
shankars99 wants to merge 3 commits intosmartcontractkit:mainfrom
shankars99:main
Open

The fixes to issue #19 #20
shankars99 wants to merge 3 commits intosmartcontractkit:mainfrom
shankars99:main

Conversation

@shankars99
Copy link

I hope I'm doing it right, it's like my second PR.

I changed read -ra args to read -r args and it seemed to fix all my problems

@PatrickAlphaC
Copy link
Contributor

Thanks for the PR! What was the issue that this fixed?

@shankars99
Copy link
Author

On my local computer and on gitpod when deploying contracts the code in the repository worked for contracts that only had 1 constructor.

Case 1. When there are no constructors parameters i.e. constructor() {}
What happened : Errored out as the command is forge create ./src/${Test}.sol:${Test} -i --rpc-url $rpc --constructor-args instead of forge create ./src/${Test}.sol:${Test} -i --rpc-url $rpc

Reason: the command expects the constructor to take in parameters that are supplied by --constructor-args. So there is the forge create that thinks the constructor has arguments as --constructor-args is passed to it.

Fix: Added an if statement to check if the parameters supplied to forge create for the constructor is 0 or not.
If 0 the forge create command executes without --constructor-args.


Case 2. More than 1 constructor parameters i.e. constructor(uint256 a, string calldata b, ...) {}

What happened : read -ra stores the args passes as an array, but my terminal and gitpod use -rA instead of -ra so this could be a wider issue.

Reason : With read -rA ${args[@]} should be used for all the parameters to be passed in forge create making the command
forge create ./src/${Test}.sol:${Test} -i --rpc-url $rpc --constructor-args ${args[@]}

Fix: Using read -r instead stores all the parameters passed as a string that can be directly passed as ${args}

@PatrickAlphaC
Copy link
Contributor

Interesting... Yeah... Do you know why your terminal isn't using -ra? It seems really odd that it's doing -rA. Using -ra should cover both of those since it'll just dump the args into an empty array.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants