Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fee properties to result data structure #57

Closed
rndquu opened this issue Jul 13, 2024 · 13 comments · Fixed by #159
Closed

Add fee properties to result data structure #57

rndquu opened this issue Jul 13, 2024 · 13 comments · Fixed by #159

Comments

@rndquu
Copy link
Member

rndquu commented Jul 13, 2024

this looks good but I have another idea:

how about we add another variable fee (percentage) and totalAfterFee/totalBeforeFee and we leave other values unmodified? this way we can display a comment which includes information about the total reward, how much was the fee and total reward after the fee.

This would increase transparency so the user can clearly see how much the comments/task was worth and sees how much was the fee

Originally posted by @whilefoo in #43 (review)

@rndquu
Copy link
Member Author

rndquu commented Jul 15, 2024

@whilefoo @gentlementlegen

So this issue implies refactoring the result data structure to be (example):

{
  "userName": {
    "fee": 10, # percentage
    "totalBeforeFee": 1000,
    "totalAfterFee": 900,
    ...other properties

Sounds good?

@gentlementlegen
Copy link
Member

@rndquu Will you keep the total as well? The sum currently takes all the rewards within the data and puts them inside of total
https://github.com/ubiquibot/conversation-rewards/blob/development/src/parser/processor.ts#L78
so keep this in mind when you name the elements. Otherwise good with the idea.

@gentlementlegen
Copy link
Member

@rndquu I suppose only fee should be required? If it is just a percentage the before and after should be easy to figure out. And maybe we should rename it feePercentage for clarity?

@gentlementlegen
Copy link
Member

/start

Copy link
Contributor

ubiquity-os bot commented Oct 12, 2024

! You have reached your max task limit. Please close out some tasks before assigning new ones.

1 similar comment
Copy link
Contributor

! You have reached your max task limit. Please close out some tasks before assigning new ones.

@gentlementlegen
Copy link
Member

I checked the configuration and the max assigned tasks is 2. I am assigned to three, but opened a PR against two of them, shouldn't I be allowed to start? @0x4007

@gentlementlegen gentlementlegen self-assigned this Oct 12, 2024
Copy link
Contributor

@gentlementlegen the deadline is at Sat, Oct 12, 6:55 AM UTC

1 similar comment
Copy link
Contributor

ubiquity-os bot commented Oct 12, 2024

@gentlementlegen the deadline is at Sat, Oct 12, 6:55 AM UTC

@0x4007
Copy link
Member

0x4007 commented Oct 12, 2024

I checked the configuration and the max assigned tasks is 2. I am assigned to three, but opened a PR against two of them, shouldn't I be allowed to start? @0x4007

There's an issue Whilefoo filed that outlines what we are still missing from the original implementation.

Basically after a 24 hour reviewer timeout you get an extra task

After any pull is approved by one reviewer you get an extra task

I think there was another condition but I'm forgetting at the moment.


My start attempts were also being rejected and I did a graphql call to see my five assigned issues.

Three were in the sandbox

One was in an archived repository from four years ago

So basically we need to only search from what tasks are inside of the DevPool directory, as that would have solved this problem. I believe I filed an issue for that as well.

Copy link
Contributor

ubiquity-os-beta bot commented Oct 16, 2024

 [ 50 WXDAI ] 

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueTask150
IssueComment30
ReviewComment70
Conversation Incentives
CommentFormattingRelevanceReward
@rndquu Will you keep the `total` as well? The sum curre…
0
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 49
  wordValue: 0
  result: 0
0.850
@rndquu I suppose only `fee` should be required? If it i…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 31
  wordValue: 0
  result: 0
0.80
I checked the configuration and the max assigned tasks is 2. I a…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 32
  wordValue: 0
  result: 0
0.30
Resolves #57 QA: https://github.com/Meniole/conversation-rewar…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 13
  wordValue: 0
  result: 0
0.70
It is in the linked QA:<img width="508" alt="image" src="ht…
0
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 35
  wordValue: 0
  result: 0
0.50
It's to avoid a name clash with the one that is sent to the perm…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 21
  wordValue: 0
  result: 0
0.40
These are workflow inputs not environment variables, they should…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 17
  wordValue: 0
  result: 0
0.80
At some point cspell wouldn't want to pickup its configuration, …
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 27
  wordValue: 0
  result: 0
0.30
@0x4007 Let me know if you are okay with the display and I will …
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 18
  wordValue: 0
  result: 0
0.20
New QA: https://github.com/Meniole/conversation-rewards/issues/2…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 21
  wordValue: 0
  result: 0
0.60

 [ 19.131 WXDAI ] 

@rndquu
Contributions Overview
ViewContributionCountReward
IssueSpecification113.23
IssueComment15.901
Conversation Incentives
CommentFormattingRelevanceReward
this looks good but I have another idea: how about we add anot…
4.41
content:
  content:
    p:
      score: 0
      elementCount: 4
    em:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 86
  wordValue: 0.1
  result: 4.41
113.23
@whilefoo @gentlementlegen So this issue implies refactoring t…
6.06
content:
  content:
    p:
      score: 0
      elementCount: 3
    a:
      score: 5
      elementCount: 1
  result: 5
regex:
  wordCount: 16
  wordValue: 0.1
  result: 1.06
0.855.901

 [ 16.616 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment15.151
ReviewComment911.465
Conversation Incentives
CommentFormattingRelevanceReward
There's an issue Whilefoo filed that outlines what we are still …
6.93
content:
  content:
    h2:
      score: 1
      elementCount: 1
    p:
      score: 0
      elementCount: 1
  result: 1
regex:
  wordCount: 122
  wordValue: 0.1
  result: 5.93
0.75.151
Cspell changes don't make sense to me
0.59
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 8
  wordValue: 0.1
  result: 0.59
0.10.059
```suggestionAPP_ID: ${{ secrets.APP_ID …
0.39
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 5
  wordValue: 0.1
  result: 0.39
0.80.312
This is interesting. Unsure if this is the most presentable way.…
8.4
content:
  content:
    p:
      score: 0
      elementCount: 2
    h6:
      score: 1
      elementCount: 1
    a:
      score: 5
      elementCount: 1
  result: 6
regex:
  wordCount: 42
  wordValue: 0.1
  result: 2.4
0.67.44
```suggestionimport logger from "../helpers/logger…
0.94
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 14
  wordValue: 0.1
  result: 0.94
0.50.47
I think it makes sense to keep the references to the same values…
1.33
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 21
  wordValue: 0.1
  result: 1.33
0.70.931
Why did you change this logic?
0.46
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 6
  wordValue: 0.1
  result: 0.46
0.20.092
Renaming doesn't seem right
0.39
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 5
  wordValue: 0.1
  result: 0.39
0.30.117
Thanks for the QAAs part of a separate task, I think we should …
1.8
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 30
  wordValue: 0.1
  result: 1.8
0.91.62
Just noticed the committed compiled js. I'm assuming this is nec…
1.06
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 16
  wordValue: 0.1
  result: 1.06
0.40.424

Copy link
Contributor

ubiquity-os bot commented Oct 16, 2024

 [ 50 WXDAI ] 

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueTask150
IssueComment30
ReviewComment70
Conversation Incentives
CommentFormattingRelevanceReward
@rndquu Will you keep the `total` as well? The sum curre…
0
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 49
  wordValue: 0
  result: 0
0.850
@rndquu I suppose only `fee` should be required? If it i…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 31
  wordValue: 0
  result: 0
0.90
I checked the configuration and the max assigned tasks is 2. I a…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 32
  wordValue: 0
  result: 0
0.40
Resolves #57 QA: https://github.com/Meniole/conversation-rewar…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 13
  wordValue: 0
  result: 0
0.50
It is in the linked QA:<img width="508" alt="image" src="ht…
0
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 35
  wordValue: 0
  result: 0
0.70
It's to avoid a name clash with the one that is sent to the perm…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 21
  wordValue: 0
  result: 0
0.60
These are workflow inputs not environment variables, they should…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 17
  wordValue: 0
  result: 0
0.80
At some point cspell wouldn't want to pickup its configuration, …
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 27
  wordValue: 0
  result: 0
0.40
@0x4007 Let me know if you are okay with the display and I will …
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 18
  wordValue: 0
  result: 0
0.40
New QA: https://github.com/Meniole/conversation-rewards/issues/2…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 21
  wordValue: 0
  result: 0
0.60

 [ 19.184 WXDAI ] 

@rndquu
Contributions Overview
ViewContributionCountReward
IssueSpecification113.23
IssueComment15.954
Conversation Incentives
CommentFormattingRelevanceReward
this looks good but I have another idea: how about we add anot…
4.41
content:
  content:
    p:
      score: 0
      elementCount: 4
    em:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 86
  wordValue: 0.1
  result: 4.41
113.23
@whilefoo @gentlementlegen So this issue implies refactoring t…
6.06
content:
  content:
    p:
      score: 0
      elementCount: 3
    a:
      score: 5
      elementCount: 1
  result: 5
regex:
  wordCount: 16
  wordValue: 0.1
  result: 1.06
0.95.954

 [ 14.859 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment14.558
ReviewComment910.301
Conversation Incentives
CommentFormattingRelevanceReward
There's an issue Whilefoo filed that outlines what we are still …
6.93
content:
  content:
    h2:
      score: 1
      elementCount: 1
    p:
      score: 0
      elementCount: 1
  result: 1
regex:
  wordCount: 122
  wordValue: 0.1
  result: 5.93
0.64.558
Cspell changes don't make sense to me
0.59
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 8
  wordValue: 0.1
  result: 0.59
0.10.059
```suggestionAPP_ID: ${{ secrets.APP_ID …
0.39
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 5
  wordValue: 0.1
  result: 0.39
0.70.273
This is interesting. Unsure if this is the most presentable way.…
8.4
content:
  content:
    p:
      score: 0
      elementCount: 2
    h6:
      score: 1
      elementCount: 1
    a:
      score: 5
      elementCount: 1
  result: 6
regex:
  wordCount: 42
  wordValue: 0.1
  result: 2.4
0.57.2
```suggestionimport logger from "../helpers/logger…
0.94
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 14
  wordValue: 0.1
  result: 0.94
0.60.564
I think it makes sense to keep the references to the same values…
1.33
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 21
  wordValue: 0.1
  result: 1.33
0.81.064
Why did you change this logic?
0.46
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 6
  wordValue: 0.1
  result: 0.46
0.20.092
Renaming doesn't seem right
0.39
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 5
  wordValue: 0.1
  result: 0.39
0.30.117
Thanks for the QAAs part of a separate task, I think we should …
1.8
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 30
  wordValue: 0.1
  result: 1.8
0.40.72
Just noticed the committed compiled js. I'm assuming this is nec…
1.06
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 16
  wordValue: 0.1
  result: 1.06
0.20.212

Copy link
Contributor

ubiquity-os-beta bot commented Oct 23, 2024

 [ 50 WXDAI ] 

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueTask150
IssueComment30
ReviewComment80
Conversation Incentives
CommentFormattingRelevanceReward
@rndquu Will you keep the `total` as well? The sum curre…
0
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 49
  wordValue: 0
  result: 0
0.850
@rndquu I suppose only `fee` should be required? If it i…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 31
  wordValue: 0
  result: 0
0.90
I checked the configuration and the max assigned tasks is 2. I a…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 32
  wordValue: 0
  result: 0
0.40
Resolves #57 QA: https://github.com/Meniole/conversation-rewar…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 13
  wordValue: 0
  result: 0
0.60
It is in the linked QA:<img width="508" alt="image" src="ht…
0
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 35
  wordValue: 0
  result: 0
0.40
It's to avoid a name clash with the one that is sent to the perm…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 21
  wordValue: 0
  result: 0
0.50
These are workflow inputs not environment variables, they should…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 17
  wordValue: 0
  result: 0
0.80
At some point cspell wouldn't want to pickup its configuration, …
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 27
  wordValue: 0
  result: 0
0.30
@0x4007 Let me know if you are okay with the display and I will …
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 18
  wordValue: 0
  result: 0
0.20
New QA: https://github.com/Meniole/conversation-rewards/issues/2…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 21
  wordValue: 0
  result: 0
0.50
@0x4007 Yes otherwise we should have to compile it. I'll have a …
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 22
  wordValue: 0
  result: 0
0.40

 [ 19.131 WXDAI ] 

@rndquu
Contributions Overview
ViewContributionCountReward
IssueSpecification113.23
IssueComment15.901
Conversation Incentives
CommentFormattingRelevanceReward
this looks good but I have another idea: how about we add anot…
4.41
content:
  content:
    p:
      score: 0
      elementCount: 4
    em:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 86
  wordValue: 0.1
  result: 4.41
113.23
@whilefoo @gentlementlegen So this issue implies refactoring t…
6.06
content:
  content:
    p:
      score: 0
      elementCount: 3
    a:
      score: 5
      elementCount: 1
  result: 5
regex:
  wordCount: 16
  wordValue: 0.1
  result: 1.06
0.855.901

 [ 15.438 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment14.558
ReviewComment910.88
Conversation Incentives
CommentFormattingRelevanceReward
There's an issue Whilefoo filed that outlines what we are still …
6.93
content:
  content:
    h2:
      score: 1
      elementCount: 1
    p:
      score: 0
      elementCount: 1
  result: 1
regex:
  wordCount: 122
  wordValue: 0.1
  result: 5.93
0.64.558
Cspell changes don't make sense to me
0.59
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 8
  wordValue: 0.1
  result: 0.59
0.10.059
```suggestionAPP_ID: ${{ secrets.APP_ID …
0.39
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 5
  wordValue: 0.1
  result: 0.39
0.80.312
This is interesting. Unsure if this is the most presentable way.…
8.4
content:
  content:
    p:
      score: 0
      elementCount: 2
    h6:
      score: 1
      elementCount: 1
    a:
      score: 5
      elementCount: 1
  result: 6
regex:
  wordCount: 42
  wordValue: 0.1
  result: 2.4
0.57.2
```suggestionimport logger from "../helpers/logger…
0.94
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 14
  wordValue: 0.1
  result: 0.94
0.70.658
I think it makes sense to keep the references to the same values…
1.33
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 21
  wordValue: 0.1
  result: 1.33
0.70.931
Why did you change this logic?
0.46
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 6
  wordValue: 0.1
  result: 0.46
0.30.138
Renaming doesn't seem right
0.39
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 5
  wordValue: 0.1
  result: 0.39
0.20.078
Thanks for the QAAs part of a separate task, I think we should …
1.8
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 30
  wordValue: 0.1
  result: 1.8
0.61.08
Just noticed the committed compiled js. I'm assuming this is nec…
1.06
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 16
  wordValue: 0.1
  result: 1.06
0.40.424

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants