Test token app #157

2019-07-29T12:28:00Z

haythem96 commented

2019-07-29 12:28:00 +00:00

(Migrated from github.com)

This PR add unit tests for the Token smart contracts. #103

haythem96 commented

2019-07-31 13:00:41 +00:00

(Migrated from github.com)

can we merge this one @bumi ?

bumi (Migrated from github.com) approved these changes 2019-07-31 14:02:35 +00:00

bumi (Migrated from github.com) left a comment

looks good to me. :shipit:
see that one comment, then we can merge it.

looks good to me. :shipit: see that one comment, then we can merge it.

apps/token/contracts/Token.sol

bumi (Migrated from github.com) commented

2019-07-31 14:00:46 +00:00

I think so far I always used keys like INVALID_AMOUNT instead of full messages.
This should make the code smaller and is enough to identify the error and potentially also map it to a full message

I think so far I always used keys like `INVALID_AMOUNT` instead of full messages. This should make the code smaller and is enough to identify the error and potentially also map it to a full message

haythem96 (Migrated from github.com) reviewed 2019-07-31 14:46:56 +00:00

apps/token/contracts/Token.sol

						
				@@ -24,2 +24,4 @@

				    require(amount > 0, "INVALID_AMOUNT");

				    uint256 amountInWei = amount.mul(1 ether);

				    _mint(contributorAccount, amountInWei);

haythem96 (Migrated from github.com) commented

2019-07-31 14:46:56 +00:00

@bumi is that cool ?

bumi (Migrated from github.com) reviewed 2019-07-31 15:01:46 +00:00

apps/token/contracts/Token.sol

						
				@@ -24,2 +24,4 @@

				    require(amount > 0, "INVALID_AMOUNT");

				    uint256 amountInWei = amount.mul(1 ether);

				    _mint(contributorAccount, amountInWei);

bumi (Migrated from github.com) commented

2019-07-31 15:01:46 +00:00

just saw that I haven't done it everywhere. I thought about something like this: https://github.com/67P/kredits-contracts/blob/master/apps/contribution/contracts/Contribution.sol#L181

as that full string increases the contract and thus the deployment costs, afaik? so I thought it is better to use small strings.

just saw that I haven't done it everywhere. I thought about something like this: https://github.com/67P/kredits-contracts/blob/master/apps/contribution/contracts/Contribution.sol#L181 as that full string increases the contract and thus the deployment costs, afaik? so I thought it is better to use small strings.

haythem96 (Migrated from github.com) reviewed 2019-07-31 15:03:30 +00:00

apps/token/contracts/Token.sol

						
				@@ -24,2 +24,4 @@

				    require(amount > 0, "INVALID_AMOUNT");

				    uint256 amountInWei = amount.mul(1 ether);

				    _mint(contributorAccount, amountInWei);

haythem96 (Migrated from github.com) commented

2019-07-31 15:03:30 +00:00

ahh okaay (y)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kredits/contracts#157