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

Do not unassign users that have activity on a pull-request #21

Closed
gentlementlegen opened this issue Sep 13, 2024 · 16 comments
Closed

Do not unassign users that have activity on a pull-request #21

gentlementlegen opened this issue Sep 13, 2024 · 16 comments

Comments

@gentlementlegen
Copy link
Member

gentlementlegen commented Sep 13, 2024

          I think there is an issue with the unassign. It doesn't seem to take into account linked pull-request activity, I got unassigned but opened my PR 12 hours ago. @0x4007 @Keyrxng rfc

Originally posted by @gentlementlegen in ubiquity-os-marketplace/text-conversation-rewards#82 (comment)

Users that opened and linked a pull-request seem to be unassigned regardless of their activity. This should be taken into account before unassigning users.

Another example: ubiquity/ubiquity-dollar#563 (comment)

@0x4007
Copy link
Member

0x4007 commented Sep 13, 2024

I am skeptical that this should be funded, because this just means that the original implementation was not done correctly. The person who implemented it originally should be responsible to handle this. They can lift most of the logic from v1, aside from how we detect the linked pull is different now (just a GraphQL query needs to be adjusted.)

@Keyrxng
Copy link
Contributor

Keyrxng commented Sep 15, 2024

The issue is the rest api that is being hit is not ideal. Currently we are using rest.issues.listEvents which isn't as abundant as rest.issues.listEventsForTimeline which contains everything that we want: comments, reviews etc.

Looking at the difference in results between the two is drastic. One question I have is, should we consider a review from one of us as activity on the issue?

  • user does their job and PR awaits review
  • on the 3rd day a review is submitted
  • Should the deadline be extended from that review since that is out of the contributor's control or should we only consider a user' activity?

@Keyrxng Keyrxng self-assigned this Sep 15, 2024
@Keyrxng
Copy link
Contributor

Keyrxng commented Sep 15, 2024

I'll add the whitelist as part of this PR too

whitelist:

  • push
  • pull_request.opened
  • pull_request_review_comment.created
  • pull_request.ready_for_review
  • pull_request.review_requested
  • issue_comment.created

@Keyrxng
Copy link
Contributor

Keyrxng commented Sep 15, 2024

This is a committed event payload. Notice how you are referred to by your display name @gentlementlegen which when you try to use that with the users.getByUsername endpoint it actually returns @Mentlegen.

Because of this, the only way I can get your info is parsing the github email. I'm sure most emails are like this but idk if it's possible for these to be the real user email address and not follow the standard you see below but I cannot handle it any other way I don't think.

May encounter issues down the line but we'll need to deal with that as it happens I think.

  {
    "sha": "0a7576d65ea6845f771c9b2c26c39481743ee32c",
    "node_id": "C_kwDOLUK0B9oAKDBhNzU3NmQ2NWVhNjg0NWY3NzFjOWIyYzI2YzM5NDgxNzQzZWUzMmM",
    "url": "https://api.github.com/repos/ubiquibot/conversation-rewards/git/commits/0a7576d65ea6845f771c9b2c26c39481743ee32c",
    "html_url": "https://github.com/ubiquibot/conversation-rewards/commit/0a7576d65ea6845f771c9b2c26c39481743ee32c",
    "author": {
      "name": "Mentlegen",
      "email": "[email protected]",
      "date": "2024-09-13T15:47:10Z"
    },
    "committer": {
      "name": "Mentlegen",
      "email": "[email protected]",
      "date": "2024-09-13T15:47:10Z"
    },
    "tree": {
      "sha": "b5951a27f44b77dea078380234380f2d02ab6969",
      "url": "https://api.github.com/repos/ubiquibot/conversation-rewards/git/trees/b5951a27f44b77dea078380234380f2d02ab6969"
    },
    "message": "feat: skip minimized comments",
    "parents": [
      {
        "sha": "0799f372ae52310d0e35318ab3087e9fb8ab8f32",
        "url": "https://api.github.com/repos/ubiquibot/conversation-rewards/git/commits/0799f372ae52310d0e35318ab3087e9fb8ab8f32",
        "html_url": "https://github.com/ubiquibot/conversation-rewards/commit/0799f372ae52310d0e35318ab3087e9fb8ab8f32"
      }
    ],
    "verification": {
      "verified": false,
      "reason": "unsigned",
      "signature": null,
      "payload": null
    },
    "event": "committed"
  },

@0x4007
Copy link
Member

0x4007 commented Sep 15, 2024

One question I have is, should we consider a review from one of us as activity on the issue?

No

May encounter issues down the line but we'll need to deal with that as it happens I think.

No just keep it simple. Any commit on the pull will count as activity.

@Keyrxng Keyrxng mentioned this issue Sep 15, 2024
@Keyrxng
Copy link
Contributor

Keyrxng commented Sep 15, 2024

No just keep it simple. Any commit on the pull will count as activity.

  1. Someone other than the contributor may push.
  2. We are using the entire timeline which could mean commits across PRs so we need to scope it to the assignee.
  3. I haven't encountered a scenario yet where the email isn't the noreply format but I think it is possible it can change.

@0x4007
Copy link
Member

0x4007 commented Sep 15, 2024

  1. It's fine
  2. Check timestamp

@Keyrxng
Copy link
Contributor

Keyrxng commented Sep 15, 2024

Sorry but checking the timestamp wouldn't work in this case because if there are two pulls within 3.5 - 7 days of each other then the activity would be wrong. For example pay.ubq.fi just had a contributor open three PRs within a day or two for the same issue, if there was one other contributor in the mix in the last week it wouldn't be accurate for either.

Unless we refactor and we pull in the PR separate from the issue and parse each timeline individually and merge them. Although My PR has that reach already with access to committed events only present in PRs. Someone other than the assignee might push but that's okay but it'll still be easier to attribute commits to PRs specifically.

@0x4007
Copy link
Member

0x4007 commented Sep 15, 2024

It worked fine in v1 bot. And I'm pretty sure it was as simple as checking any linked pull for any commits. Then the assignee gets extra time.

You don't need to make it more complicated than it needs to be.

@Keyrxng
Copy link
Contributor

Keyrxng commented Sep 15, 2024

We already collect the linked pull request and issue timeline and merged actually I was mistaken but I will refactor the PR so that it counts any commit as activity.

Copy link
Contributor

ubiquity-os bot commented Sep 19, 2024

@Keyrxng, this task has been idle for a while. Please provide an update.

1 similar comment
Copy link
Contributor

ubiquity-os bot commented Sep 23, 2024

@Keyrxng, this task has been idle for a while. Please provide an update.

@0x4007 0x4007 closed this as completed in 54f0e06 Oct 10, 2024
Copy link
Contributor

ubiquity-os bot commented Oct 10, 2024

 [ 100 WXDAI ] 

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueTask1100
IssueComment60
Conversation Incentives
CommentFormattingRelevanceReward
The issue is the rest api that is being hit is not ideal. Curren…
0
content:
  content: {}
  result: 0
regex:
  wordCount: 30
  wordValue: 0
  result: 0
0.9-
I'll add the whitelist as part of this PR toowhitelist:- …
0
content:
  content: {}
  result: 0
regex:
  wordCount: 1
  wordValue: 0
  result: 0
0.8-
This is a `committed` event payload. Notice how you are …
0
content:
  content: {}
  result: 0
regex:
  wordCount: 59
  wordValue: 0
  result: 0
0.7-
1. Someone other than the contributor may push.2. We are using…
0
content:
  content: {}
  result: 0
regex:
  wordCount: 0
  wordValue: 0
  result: 0
0.6-
Sorry but checking the timestamp wouldn't work in this case beca…
0
content:
  content: {}
  result: 0
regex:
  wordCount: 59
  wordValue: 0
  result: 0
0.5-
We already collect the linked pull request and issue timeline an…
0
content:
  content: {}
  result: 0
regex:
  wordCount: 0
  wordValue: 0
  result: 0
0.85-

 [ 0 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment40
Conversation Incentives
CommentFormattingRelevanceReward
I am skeptical that this should be funded, because this just mea…
0
content:
  content: {}
  result: 0
regex:
  wordCount: 0
  wordValue: 0
  result: 0
0.8-
NoNo just keep it simple. Any commit on the pull will count as …
0
content:
  content: {}
  result: 0
regex:
  wordCount: 0
  wordValue: 0
  result: 0
0.7-
1. It's fine2. Check timestamp
0
content:
  content: {}
  result: 0
regex:
  wordCount: 0
  wordValue: 0
  result: 0
0.6-
It worked fine in v1 bot. And I'm pretty sure it was as simple a…
0
content:
  content: {}
  result: 0
regex:
  wordCount: 0
  wordValue: 0
  result: 0
0.9-

 [ 13.23 WXDAI ] 

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueSpecification113.23
Conversation Incentives
CommentFormattingRelevanceReward
I think there is an issue with the unassign. It doesn't seem to …
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

Copy link

ubiquity-os-beta bot commented Oct 12, 2024

 [ 100 WXDAI ] 

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueTask1100
IssueComment60
Conversation Incentives
CommentFormattingRelevanceReward
The issue is the rest api that is being hit is not ideal. Curren…
2.5
content:
  content:
    p:
      score: 0
      elementCount: 5
    ul:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 3
  result: 2.5
regex:
  wordCount: 103
  wordValue: 0
  result: 0
0.70
I'll add the whitelist as part of this PR toowhitelist:- …
4
content:
  content:
    p:
      score: 0
      elementCount: 8
    ul:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 6
  result: 4
regex:
  wordCount: 12
  wordValue: 0
  result: 0
0.60
This is a `committed` event payload. Notice how you are …
0
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 108
  wordValue: 0
  result: 0
0.50
1. Someone other than the contributor may push.2. We are using…
2.5
content:
  content:
    ol:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 3
    p:
      score: 0
      elementCount: 3
  result: 2.5
regex:
  wordCount: 52
  wordValue: 0
  result: 0
0.550
Sorry but checking the timestamp wouldn't work in this case beca…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 128
  wordValue: 0
  result: 0
0.650
We already collect the linked pull request and issue timeline an…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 30
  wordValue: 0
  result: 0
0.750

 [ 7.514 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment47.514
Conversation Incentives
CommentFormattingRelevanceReward
I am skeptical that this should be funded, because this just mea…
3.25
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 60
  wordValue: 0.1
  result: 3.25
0.72.275
NoNo just keep it simple. Any commit on the pull will count as …
1
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 15
  wordValue: 0.1
  result: 1
0.80.8
1. It's fine2. Check timestamp
2.39
content:
  content:
    ol:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 2
  result: 2
regex:
  wordCount: 5
  wordValue: 0.1
  result: 0.39
0.62.234
It worked fine in v1 bot. And I'm pretty sure it was as simple a…
2.45
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 43
  wordValue: 0.1
  result: 2.45
0.92.205

 [ 13.23 WXDAI ] 

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueSpecification113.23
Conversation Incentives
CommentFormattingRelevanceReward
I think there is an issue with the unassign. It doesn't seem to …
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

Copy link
Contributor

ubiquity-os bot commented Oct 12, 2024

 [ 100 WXDAI ] 

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueTask1100
IssueComment60
Conversation Incentives
CommentFormattingRelevanceReward
The issue is the rest api that is being hit is not ideal. Curren…
2.5
content:
  content:
    p:
      score: 0
      elementCount: 5
    ul:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 3
  result: 2.5
regex:
  wordCount: 103
  wordValue: 0
  result: 0
0.850
I'll add the whitelist as part of this PR toowhitelist:- …
4
content:
  content:
    p:
      score: 0
      elementCount: 8
    ul:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 6
  result: 4
regex:
  wordCount: 12
  wordValue: 0
  result: 0
0.750
This is a `committed` event payload. Notice how you are …
0
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 108
  wordValue: 0
  result: 0
0.80
1. Someone other than the contributor may push.2. We are using…
2.5
content:
  content:
    ol:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 3
    p:
      score: 0
      elementCount: 3
  result: 2.5
regex:
  wordCount: 52
  wordValue: 0
  result: 0
0.70
Sorry but checking the timestamp wouldn't work in this case beca…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 128
  wordValue: 0
  result: 0
0.650
We already collect the linked pull request and issue timeline an…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 30
  wordValue: 0
  result: 0
0.60

 [ 7.739 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment47.739
Conversation Incentives
CommentFormattingRelevanceReward
I am skeptical that this should be funded, because this just mea…
3.25
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 60
  wordValue: 0.1
  result: 3.25
0.82.6
NoNo just keep it simple. Any commit on the pull will count as …
1
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 15
  wordValue: 0.1
  result: 1
0.70.7
1. It's fine2. Check timestamp
2.39
content:
  content:
    ol:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 2
  result: 2
regex:
  wordCount: 5
  wordValue: 0.1
  result: 0.39
0.62.234
It worked fine in v1 bot. And I'm pretty sure it was as simple a…
2.45
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 43
  wordValue: 0.1
  result: 2.45
0.92.205

 [ 13.23 WXDAI ] 

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueSpecification113.23
Conversation Incentives
CommentFormattingRelevanceReward
I think there is an issue with the unassign. It doesn't seem to …
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

Copy link

ubiquity-os-beta bot commented Oct 23, 2024

 [ 100 WXDAI ] 

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueTask1100
IssueComment60
Conversation Incentives
CommentFormattingRelevanceReward
The issue is the rest api that is being hit is not ideal. Curren…
2.5
content:
  content:
    p:
      score: 0
      elementCount: 5
    ul:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 3
  result: 2.5
regex:
  wordCount: 103
  wordValue: 0
  result: 0
0.80
I'll add the whitelist as part of this PR toowhitelist:- …
4
content:
  content:
    p:
      score: 0
      elementCount: 8
    ul:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 6
  result: 4
regex:
  wordCount: 12
  wordValue: 0
  result: 0
0.750
This is a `committed` event payload. Notice how you are …
0
content:
  content:
    p:
      score: 0
      elementCount: 3
  result: 0
regex:
  wordCount: 108
  wordValue: 0
  result: 0
0.60
1. Someone other than the contributor may push.2. We are using…
2.5
content:
  content:
    ol:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 3
    p:
      score: 0
      elementCount: 3
  result: 2.5
regex:
  wordCount: 52
  wordValue: 0
  result: 0
0.650
Sorry but checking the timestamp wouldn't work in this case beca…
0
content:
  content:
    p:
      score: 0
      elementCount: 2
  result: 0
regex:
  wordCount: 128
  wordValue: 0
  result: 0
0.70
We already collect the linked pull request and issue timeline an…
0
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 30
  wordValue: 0
  result: 0
0.850

 [ 7.689 WXDAI ] 

@0x4007
Contributions Overview
ViewContributionCountReward
IssueComment47.689
Conversation Incentives
CommentFormattingRelevanceReward
I am skeptical that this should be funded, because this just mea…
3.25
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 60
  wordValue: 0.1
  result: 3.25
0.82.6
NoNo just keep it simple. Any commit on the pull will count as …
1
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 15
  wordValue: 0.1
  result: 1
0.650.65
1. It's fine2. Check timestamp
2.39
content:
  content:
    ol:
      score: 1
      elementCount: 1
    li:
      score: 0.5
      elementCount: 2
  result: 2
regex:
  wordCount: 5
  wordValue: 0.1
  result: 0.39
0.62.234
It worked fine in v1 bot. And I'm pretty sure it was as simple a…
2.45
content:
  content:
    p:
      score: 0
      elementCount: 1
  result: 0
regex:
  wordCount: 43
  wordValue: 0.1
  result: 2.45
0.92.205

 [ 13.23 WXDAI ] 

@gentlementlegen
Contributions Overview
ViewContributionCountReward
IssueSpecification113.23
Conversation Incentives
CommentFormattingRelevanceReward
I think there is an issue with the unassign. It doesn't seem to …
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

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