kredits/kredits-web
9
0
Fork 0
Code Issues 22 Pull Requests 1 Actions Releases 18 Activity

Switch to contribution token instead of proposals #100

Merged
raucao merged 5 commits from feature/20-contributions into master 2019-04-09 20:25:36 +00:00
Conversation 12 Commits 5 Files Changed 121
+4160 -16582
raucao commented 2019-04-09 10:57:32 +00:00 (Migrated from github.com)
Copy Link
Copy Source

No vetos yet, and only for collections (not creation).

Also removes Bourbon and Neat in favor of plain Sass.

closes #20

No vetos yet, and only for collections (not creation). Also removes Bourbon and Neat in favor of plain Sass. closes #20
bumi commented 2019-04-09 14:21:39 +00:00 (Migrated from github.com)
Copy Link
Copy Source

utACK

utACK
raucao commented 2019-04-09 15:18:37 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Btw, you guys can test this by simply running npm i && npm start (with local IPFS daemon running). That will show data from Rinkeby.

Btw, you guys can test this by simply running `npm i && npm start` (with local IPFS daemon running). That will show data from Rinkeby.
bumi (Migrated from github.com) requested changes 2019-04-09 20:06:07 +00:00
app/controllers/index.js
@@ -12,26 +14,44 @@ export default Controller.extend({
contributorsSorting: Object.freeze(['balance:desc']),
bumi (Migrated from github.com) commented 2019-04-09 20:03:10 +00:00
Copy Link
Copy Source

I think this should be the other way round.
unconfirmed = confirmedAt > currentBlock

or:
if the currentBlock is smaller than the block number in confirmedAt then the contribution is not confirmed.

I think this should be the other way round. unconfirmed = confirmedAt > currentBlock or: if the currentBlock is smaller than the block number in confirmedAt then the contribution is not confirmed.
bumi (Migrated from github.com) commented 2019-04-09 20:03:48 +00:00
Copy Link
Copy Source
      return this.currentBlock >= contribution.confirmedAt;
```suggestion return this.currentBlock >= contribution.confirmedAt; ```
bumi (Migrated from github.com) commented 2019-04-09 20:04:16 +00:00
Copy Link
Copy Source
      return this.currentBlock < contribution.confirmedAt;
```suggestion return this.currentBlock < contribution.confirmedAt; ```
app/services/kredits.js
bumi (Migrated from github.com) commented 2019-04-09 20:05:29 +00:00
Copy Link
Copy Source

do we actually need to await this here?
currentBlock is not used in this function it seems?

do we actually need to await this here? currentBlock is not used in this function it seems?
raucao (Migrated from github.com) reviewed 2019-04-09 20:14:04 +00:00
app/services/kredits.js
raucao (Migrated from github.com) commented 2019-04-09 20:14:04 +00:00
Copy Link
Copy Source

We don't want to set it to a promise I think.

We don't want to set it to a promise I think.
raucao (Migrated from github.com) reviewed 2019-04-09 20:14:22 +00:00
app/controllers/index.js
@@ -12,26 +14,44 @@ export default Controller.extend({
contributorsSorting: Object.freeze(['balance:desc']),
raucao (Migrated from github.com) commented 2019-04-09 20:14:21 +00:00
Copy Link
Copy Source

Aw crap, of course. Brain fart.

Aw crap, of course. Brain fart.
bumi (Migrated from github.com) reviewed 2019-04-09 20:14:59 +00:00
package.json
@@ -16,3 +16,3 @@
"start": "ember serve",
"test": "ember test",
"start": "KREDITS_APM_DOMAIN=open.aragonpm.eth ember serve",
"start:local": "WEB3_PROVIDER_URL=http://localhost:7545 ember s",
bumi (Migrated from github.com) commented 2019-04-09 20:14:59 +00:00
Copy Link
Copy Source

👍

:+1:
bumi (Migrated from github.com) reviewed 2019-04-09 20:18:54 +00:00
app/services/kredits.js
bumi (Migrated from github.com) commented 2019-04-09 20:18:54 +00:00
Copy Link
Copy Source

ok, I thought we want to set a promise that ember automatically resolves... like getContributions etc. return promises.

ok, I thought we want to set a promise that ember automatically resolves... like `getContributions` etc. return promises.
bumi commented 2019-04-09 20:20:08 +00:00 (Migrated from github.com)
Copy Link
Copy Source

we soon also need to load the balances from the confirmed contributions or add more information on what we show. otherwise it is a bit misleading.

we soon also need to load the balances from the confirmed contributions or add more information on what we show. otherwise it is a bit misleading.
raucao (Migrated from github.com) reviewed 2019-04-09 20:22:00 +00:00
app/services/kredits.js
raucao (Migrated from github.com) commented 2019-04-09 20:22:00 +00:00
Copy Link
Copy Source

That's different. getContributions doesn't set a property directly. Wherever it's used, the promise is actually handled.

That's different. `getContributions` doesn't set a property directly. Wherever it's used, the promise is actually handled.
raucao commented 2019-04-09 20:23:27 +00:00 (Migrated from github.com)
Copy Link
Copy Source

we soon also need to load the balances from the confirmed contributions or add more information on what we show. otherwise it is a bit misleading.

Yes, but we currently don't get the correct balance of kredits earned via getContributors. It's a new issue, but I agree that it is the next urgent one.

> we soon also need to load the balances from the confirmed contributions or add more information on what we show. otherwise it is a bit misleading. Yes, but we currently don't get the correct balance of kredits earned via `getContributors`. It's a new issue, but I agree that it is the next urgent one.
bumi (Migrated from github.com) approved these changes 2019-04-09 20:23:56 +00:00
raucao (Migrated from github.com) reviewed 2019-04-09 20:24:42 +00:00
app/services/kredits.js
raucao (Migrated from github.com) commented 2019-04-09 20:24:42 +00:00
Copy Link
Copy Source

(That is to say the model hooks are promise-aware)

(That is to say the model hooks are promise-aware)
Sign in to join this conversation.
Reviewers
No Reviewers
bumi
raucao
Labels
Clear labels
bug
Something is not working
 dev environment
Config, builds, CI, deployment, etc.
 docs
Documentation
 duplicate
This issue or pull request already exists
 enhancement
Improving existing functionality
 feature
New functionality
 idea
Something to consider
 invalid
Not a bug
 kredits-1
Small contribution
 kredits-2
Medium contribution
 kredits-3
Large contribution
 question
Looking for an answer
release
major
release
minor
release
patch
security
All your base are belong to us
 ui/ux
User interface, process design, etc.
 wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
bkero (bkero) bumi (bumi) fsmanuel (fsmanuel) galfert (galfert) greg (Greg) haythem (haythem) hueso (hueso) raucao (Râu Cao) slvrbckt (Nick Jennings)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kredits/kredits-web#100
Delete Branch "feature/20-contributions"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?