Improve totalKreditsEarned #121

2019-04-27T20:04:12Z

raucao commented

2019-04-27 20:04:12 +00:00

(Migrated from github.com)

Use a more reasonable size of integer
Use better variable names (count != amount, balance != count)

* Use a more reasonable size of integer * Use better variable names (count != amount, balance != count)

bumi commented

2019-04-28 20:01:24 +00:00

(Migrated from github.com)

fyi: due to the ERC721 standard that naming is a bit confusing e.g. see the balanceOf function which does a count of contributions.
(though I am tending more and more to move away and ignore that standard)

fyi: due to the ERC721 standard that naming is a bit confusing e.g. see the `balanceOf` function which does a count of contributions. (though I am tending more and more to move away and ignore that standard)

bumi commented

2019-04-28 20:07:25 +00:00

(Migrated from github.com)

Did you test that the different returned types (uint32) don't break the scripts / kredits-web? (I am lazy right now to go through the setup :/ )

raucao commented

2019-04-28 20:16:26 +00:00

(Migrated from github.com)

Did you test that the different returned types (uint32) don't break the scripts / kredits-web?

No, but it's a simple change for both. If it does break clients, we can release a major version of the module.

> Did you test that the different returned types (uint32) don't break the scripts / kredits-web? No, but it's a simple change for both. If it does break clients, we can release a major version of the module.

raucao commented

2019-04-28 20:17:38 +00:00

(Migrated from github.com)

fyi: due to the ERC721 standard that naming is a bit confusing e.g. see the balanceOf function which does a count of contributions.

Yes, I realize that. But it doesn't mean we have to use words that mean different things for variables in unrelated functions.

> fyi: due to the ERC721 standard that naming is a bit confusing e.g. see the `balanceOf` function which does a count of contributions. Yes, I realize that. But it doesn't mean we have to use words that mean different things for variables in unrelated functions.

bumi commented

2019-05-01 18:12:54 +00:00

(Migrated from github.com)

ToDo:

update ABI

ToDo: - [x] update ABI

bumi commented

2019-06-01 10:24:13 +00:00

(Migrated from github.com)

we should also update the code to only count non-vetoed contributions and deploy that together on-chain.

raucao commented

2019-06-01 11:02:08 +00:00

(Migrated from github.com)

we should also update the code to only count non-vetoed contributions and deploy that together on-chain.

Can't see an issue about that. Sounds like a bug to me.

> we should also update the code to only count non-vetoed contributions and deploy that together on-chain. Can't see an issue about that. Sounds like a bug to me.

bumi commented

2019-06-08 16:01:27 +00:00

(Migrated from github.com)

update the rest of the code accordingly and added the resulting, updated ABI.
did not test it with kredits-web though. But I'd say let's merge and deploy this update.

update the rest of the code accordingly and added the resulting, updated ABI. did not test it with kredits-web though. But I'd say let's merge and deploy this update.

👍 1

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kredits/contracts#121