WIP: Contribution contracts tests #159

Closed
haythem96 wants to merge 20 commits from tests/contracts-contribution into master
Showing only changes of commit 3515bdfda9 - Show all commits

View File

@ -1,5 +1,4 @@
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
const namehash = require('ethers').utils.namehash;
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
// eslint-disable-next-line no-undef
const Contribution = artifacts.require("Contribution.sol");
// eslint-disable-next-line no-undef
@ -78,7 +77,6 @@ contract('Contribution app', (accounts) => {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
appsId[2] = namehash("kredits-proposal");
appsId[3] = namehash("kredits-token");
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
//get new app instance from DAO
let receipt = await dao.newAppInstance(
appsId[0],
@ -277,14 +275,12 @@ contract('Contribution app', (accounts) => {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
});
it("claim contribution", async () => {
if(contributionId < 10) {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
await timeTravel(100);
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
}
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
else {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
if(contributionId > 10) {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
await timeTravel(blocksToWait);
await mineBlock();
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
}
await mineBlock();
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
//Claim contribution
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
await contribution.claim(contributionId);
let contributionObject = await contribution.getContribution(contributionId.toNumber());
// eslint-disable-next-line no-undef
@ -299,44 +295,4 @@ contract('Contribution app', (accounts) => {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
});
});
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
describe("Veto claimed contribution", async () => {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
// eslint-disable-next-line no-undef
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
before(async () => {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
let amount = 200;
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
let contributorId = 1;
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
let hashDigest = '0x0000000000000000000000000000000000000000000000000000000000000000';
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
let hashFunction = 1;
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
let hashSize = 1;
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
const contributionIdBefore = await contribution.contributionsCount();
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
//add contribution
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
await contribution.add(amount, contributorId, hashDigest, hashFunction, hashSize, {from: root});
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
const contributionId = await contribution.contributionsCount();
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
// eslint-disable-next-line no-undef
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
assert.equal(contributionId.toNumber()-contributionIdBefore.toNumber(), true, "contribution added");
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
//Claim contribution
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
if(contributionId < 10) {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
await timeTravel(100);
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
}
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
else {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
await timeTravel(blocksToWait);
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
}
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
await mineBlock();
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
await contribution.claim(contributionId);
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
});
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
it("should revert when veto already claimed contribution", async () => {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
const contributionId = await contribution.contributionsCount();
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
return assertRevert(async () => {
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
await contribution.veto(contributionId.toNumber(), {from: root});
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
'contribution already claimed';
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
});
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
});
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
});
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
});

haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
haythem96 commented 2019-08-02 16:29:19 +00:00 (Migrated from github.com)
Review

this one revert for some reasons

this one revert for some reasons
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:39:17 +00:00 (Migrated from github.com)
Review

Looks like the expectation wasn't described here (it("does x")). It also looks like there are 3 different expectations in one block here.

Looks like the expectation wasn't described here (`it("does x")`). It also looks like there are 3 different expectations in one block here.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:09 +00:00 (Migrated from github.com)
Review

I think you can also add expectation descriptions as the last argument to assert(), in case you really want to assert multiple things in one it block.

I think you can also add expectation descriptions as the last argument to `assert()`, in case you really want to assert multiple things in one `it` block.
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
raucao commented 2019-08-02 16:40:45 +00:00 (Migrated from github.com)
Review

Reverts unexpectedly? And in which context/scenario?

Reverts unexpectedly? And in which context/scenario?
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:24:35 +00:00 (Migrated from github.com)
Review

in the scenario of claiming an added contribution

in the scenario of claiming an added contribution
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
haythem96 commented 2019-08-03 11:37:47 +00:00 (Migrated from github.com)
Review

most of those multiple assert check for the same thing, that the contribution is added.. XD

most of those multiple assert check for the same thing, that the contribution is added.. XD
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:58:16 +00:00 (Migrated from github.com)
Review

How would that work? You can only claim confirmed contributions.

How would that work? You can only claim confirmed contributions.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
raucao commented 2019-08-03 11:59:37 +00:00 (Migrated from github.com)
Review

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.

They very line I commented on asserts that the contributorId is correct, not that the contribution itself was added.
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:40:57 +00:00 (Migrated from github.com)
Review

ahhhh yup okay, missed this require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); thanks!

ahhhh yup okay, missed this ` require(block.number >= c.confirmedAtBlock, 'NOT_CLAIMABLE'); ` thanks!
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
haythem96 commented 2019-08-03 14:42:56 +00:00 (Migrated from github.com)
Review

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage

yeah true, honestly don't know if that all those asserts are necessary... maybe just for more code coverage
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
raucao commented 2019-08-03 14:46:37 +00:00 (Migrated from github.com)
Review

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.

In this case it's only important that you describe what you mean to test exactly. Currently, there's no description of what is being tested in this block.
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-06 10:37:30 +00:00 (Migrated from github.com)
Review

this should fix the claim tests, no @skddc @bumi ?

this should fix the claim tests, no @skddc @bumi ?
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:54:48 +00:00 (Migrated from github.com)
Review

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi

idk if this test is really necessary as if a contribution is claimed, so theoretically it is impossible to veto because of block number @bumi
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:55:23 +00:00 (Migrated from github.com)
Review

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc

can't figure out why this one is not working, idk what I'm missing there! @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
haythem96 commented 2019-08-09 22:56:07 +00:00 (Migrated from github.com)
Review

blocks get minted and the veto period end right away... @bumi @skddc

blocks get minted and the veto period end right away... @bumi @skddc
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.
raucao commented 2019-09-19 09:19:32 +00:00 (Migrated from github.com)
Review

Yes, the first 10 are confirmed immediately iirc.

Yes, the first 10 are confirmed immediately iirc.