kredits/contracts
kredits/contracts
9
1
Fork 0
Code Issues 21 Pull Requests Actions Projects 1 Releases 8 Activity

Add Reimbursement app #198

Merged
bumi merged 21 commits from feature/expenses into master 2021-06-02 12:25:34 +00:00
Conversation 19 Commits 21 Files Changed 33 +1 -1
1 changed files with 1 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 3f8407fa02 - Show all commits

2
apps/reimbursement/contracts/Reimbursement.sol
View File

@ -31,7 +31,7 @@ contract Reimbursement is AragonApp {
raucao commented 2020-05-29 08:54:04 +00:00 (Migrated from github.com)
Review
Copy Link
  event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);

(Although I'm not sure if something like submittedBy isn't nicer for a name there.)

```suggestion event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount); ``` (Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
raucao commented 2020-05-29 08:54:04 +00:00 (Migrated from github.com)
Review
Copy Link
  event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);

(Although I'm not sure if something like submittedBy isn't nicer for a name there.)

```suggestion event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount); ``` (Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
raucao commented 2020-05-29 09:01:08 +00:00 (Migrated from github.com)
Review
Copy Link

Maybe we should just call this get, analog to add below?

Maybe we should just call this `get`, analog to `add` below?
raucao commented 2020-05-29 09:01:08 +00:00 (Migrated from github.com)
Review
Copy Link

Maybe we should just call this get, analog to add below?

Maybe we should just call this `get`, analog to `add` below?
raucao commented 2020-05-29 09:05:54 +00:00 (Migrated from github.com)
Review
Copy Link

I think until we actually have the financial code done, we should not add this function or the claimed property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).

I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
raucao commented 2020-05-29 09:05:54 +00:00 (Migrated from github.com)
Review
Copy Link

I think until we actually have the financial code done, we should not add this function or the claimed property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).

I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
bumi commented 2020-05-29 09:31:35 +00:00 (Migrated from github.com)
Review
Copy Link

yeah, thought about that, too. but we had getContribution in the contribution contract.
do you think that's the best pattern? 👍

yeah, thought about that, too. but we had `getContribution` in the contribution contract. do you think that's the best pattern? :+1:
bumi commented 2020-05-29 09:31:35 +00:00 (Migrated from github.com)
Review
Copy Link

yeah, thought about that, too. but we had getContribution in the contribution contract.
do you think that's the best pattern? 👍

yeah, thought about that, too. but we had `getContribution` in the contribution contract. do you think that's the best pattern? :+1:
raucao commented 2020-05-29 09:34:07 +00:00 (Migrated from github.com)
Review
Copy Link

Yeah, I think it should be consistent, and contribution.add is nicer for clients than contribution.addContribution. May as well imply the scope for all functions then. (We actually throw a deprecation warning for addContribution still. :))

Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
raucao commented 2020-05-29 09:34:07 +00:00 (Migrated from github.com)
Review
Copy Link

Yeah, I think it should be consistent, and contribution.add is nicer for clients than contribution.addContribution. May as well imply the scope for all functions then. (We actually throw a deprecation warning for addContribution still. :))

Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
uint32 public blocksToWait;
event ReimbursementAdded(uint32 id, address indexed addedByAccont, uint256 amount);
raucao commented 2020-05-29 08:54:04 +00:00 (Migrated from github.com)
Review
Copy Link
  event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);

(Although I'm not sure if something like submittedBy isn't nicer for a name there.)

```suggestion event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount); ``` (Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
raucao commented 2020-05-29 09:01:08 +00:00 (Migrated from github.com)
Review
Copy Link

Maybe we should just call this get, analog to add below?

Maybe we should just call this `get`, analog to `add` below?
raucao commented 2020-05-29 09:05:54 +00:00 (Migrated from github.com)
Review
Copy Link

I think until we actually have the financial code done, we should not add this function or the claimed property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).

I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
bumi commented 2020-05-29 09:31:35 +00:00 (Migrated from github.com)
Review
Copy Link

yeah, thought about that, too. but we had getContribution in the contribution contract.
do you think that's the best pattern? 👍

yeah, thought about that, too. but we had `getContribution` in the contribution contract. do you think that's the best pattern? :+1:
raucao commented 2020-05-29 09:34:07 +00:00 (Migrated from github.com)
Review
Copy Link

Yeah, I think it should be consistent, and contribution.add is nicer for clients than contribution.addContribution. May as well imply the scope for all functions then. (We actually throw a deprecation warning for addContribution still. :))

Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
raucao commented 2020-05-29 08:54:04 +00:00 (Migrated from github.com)
Review
Copy Link
  event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);

(Although I'm not sure if something like submittedBy isn't nicer for a name there.)

```suggestion event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount); ``` (Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
raucao commented 2020-05-29 09:01:08 +00:00 (Migrated from github.com)
Review
Copy Link

Maybe we should just call this get, analog to add below?

Maybe we should just call this `get`, analog to `add` below?
raucao commented 2020-05-29 09:05:54 +00:00 (Migrated from github.com)
Review
Copy Link

I think until we actually have the financial code done, we should not add this function or the claimed property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).

I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
bumi commented 2020-05-29 09:31:35 +00:00 (Migrated from github.com)
Review
Copy Link

yeah, thought about that, too. but we had getContribution in the contribution contract.
do you think that's the best pattern? 👍

yeah, thought about that, too. but we had `getContribution` in the contribution contract. do you think that's the best pattern? :+1:
raucao commented 2020-05-29 09:34:07 +00:00 (Migrated from github.com)
Review
Copy Link

Yeah, I think it should be consistent, and contribution.add is nicer for clients than contribution.addContribution. May as well imply the scope for all functions then. (We actually throw a deprecation warning for addContribution still. :))

Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
event ReimbursementClaimed(uint32 id, uint256 amount);
event ReimbursementVetoed(uint32 id, address vetoedByAccount);

raucao commented 2020-05-29 08:54:04 +00:00 (Migrated from github.com)
Review
Copy Link
  event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);

(Although I'm not sure if something like submittedBy isn't nicer for a name there.)

```suggestion event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount); ``` (Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
raucao commented 2020-05-29 08:54:04 +00:00 (Migrated from github.com)
Review
Copy Link
  event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);

(Although I'm not sure if something like submittedBy isn't nicer for a name there.)

```suggestion event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount); ``` (Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
raucao commented 2020-05-29 09:01:08 +00:00 (Migrated from github.com)
Review
Copy Link

Maybe we should just call this get, analog to add below?

Maybe we should just call this `get`, analog to `add` below?
raucao commented 2020-05-29 09:01:08 +00:00 (Migrated from github.com)
Review
Copy Link

Maybe we should just call this get, analog to add below?

Maybe we should just call this `get`, analog to `add` below?
raucao commented 2020-05-29 09:05:54 +00:00 (Migrated from github.com)
Review
Copy Link

I think until we actually have the financial code done, we should not add this function or the claimed property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).

I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
raucao commented 2020-05-29 09:05:54 +00:00 (Migrated from github.com)
Review
Copy Link

I think until we actually have the financial code done, we should not add this function or the claimed property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).

I think until we actually have the financial code done, we should not add this function or the `claimed` property. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).
bumi commented 2020-05-29 09:31:35 +00:00 (Migrated from github.com)
Review
Copy Link

yeah, thought about that, too. but we had getContribution in the contribution contract.
do you think that's the best pattern? 👍

yeah, thought about that, too. but we had `getContribution` in the contribution contract. do you think that's the best pattern? :+1:
bumi commented 2020-05-29 09:31:35 +00:00 (Migrated from github.com)
Review
Copy Link

yeah, thought about that, too. but we had getContribution in the contribution contract.
do you think that's the best pattern? 👍

yeah, thought about that, too. but we had `getContribution` in the contribution contract. do you think that's the best pattern? :+1:
raucao commented 2020-05-29 09:34:07 +00:00 (Migrated from github.com)
Review
Copy Link

Yeah, I think it should be consistent, and contribution.add is nicer for clients than contribution.addContribution. May as well imply the scope for all functions then. (We actually throw a deprecation warning for addContribution still. :))

Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
raucao commented 2020-05-29 09:34:07 +00:00 (Migrated from github.com)
Review
Copy Link

Yeah, I think it should be consistent, and contribution.add is nicer for clients than contribution.addContribution. May as well imply the scope for all functions then. (We actually throw a deprecation warning for addContribution still. :))

Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))