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

fix: use bot penalty #749

Merged
merged 6 commits into from Sep 18, 2023
Merged

Conversation

Keyrxng
Copy link
Contributor

@Keyrxng Keyrxng commented Sep 10, 2023

Checks if the bot penalty comment created at is more than last activity, if so it returns only the delivery timeline else it will unnassign.

Resolves #681

Example https://github.com/Keyrxng/UbiquityBotTesting/issues/11#issuecomment-1712968225

For testing purposes I added the penalty comment myself to workaround not having setup the backend fully yet but I believe it shows the logic working as intended.

My issue is old enough that it was unassigning me as you'll see with the bot doing so in the interaction list above the comments, after the fix it no longer unassigns.

uses bot penalty created at
@netlify
Copy link

netlify bot commented Sep 10, 2023

Deploy Preview for ubiquibot-staging ready!

Name Link
🔨 Latest commit bf489c2
🔍 Latest deploy log https://app.netlify.com/sites/ubiquibot-staging/deploys/6505f4e4eb846900084d7d2f
😎 Deploy Preview https://deploy-preview-749--ubiquibot-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Keyrxng Keyrxng marked this pull request as ready for review September 10, 2023 23:28
set to false
@0x4007
Copy link
Member

0x4007 commented Sep 11, 2023

I believe that it makes a lot more sense to check the timestamp of when it was reopened, instead of the follow up comment.

Why did you target the follow up comment?

Copy link
Contributor

@0xcodercrane 0xcodercrane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should get the timestamp from the reopened event and recalculate the deadline @Keyrxng

vs using penalty comment
Copy link
Contributor

@wannacfuture wannacfuture left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this issue's purpose is to make bot to not unassign hunter right after manual assigning.

I can't see it on your QA, @Keyrxng
Would you mind posting a valid one?

Or we can do it in a more simple way.
https://github.com/ubiquity/ubiquibot/pull/759/files
@pavlovcik , @0xcodercrane

@Keyrxng
Copy link
Contributor Author

Keyrxng commented Sep 13, 2023

I think this issue's purpose is to make bot to not unassign hunter right after manual assigning.

I can't see it on your QA, @Keyrxng Would you mind posting a valid one?

Or we can do it in a more simple way. https://github.com/ubiquity/ubiquibot/pull/759/files @pavlovcik , @0xcodercrane

I had read the issue as the bot unassigning once an old issue has been reopened which forced the bot to unassign due to it being over the deliverable date, the manual assign being automatically unassigned thing is just a by-product of this.

After my impl the bot stopped trying to unassign me after I manually reassigned.

But by all means if your linked pr solves this issue then you take it

@0xcodercrane
Copy link
Contributor

I think this issue's purpose is to make bot to not unassign hunter right after manual assigning.

I can't see it on your QA, @Keyrxng Would you mind posting a valid one?

Or we can do it in a more simple way. https://github.com/ubiquity/ubiquibot/pull/759/files @pavlovcik , @0xcodercrane

The purpose is to reset the deadline when you reopen the closed issue + stop unassigning the hunter at that time.

@0xcodercrane
Copy link
Contributor

0xcodercrane commented Sep 14, 2023

took a look at your PR for this task. It would also work for this issue but you have been solving another issue. @wannacfuture.

To complete the issue:
1/ Would you @Keyrxng bring up @wannacfuture 's codebase into your branch? It looks better to me.
2/ The assignee should still be @Keyrxng.

@wannacfuture
Copy link
Contributor

took a look at your PR for this task. It would also work for this issue but you have been solving another issue. @wannacfuture.

the root cause of the PR's related issue also contains fixing deadline.

@Keyrxng
Copy link
Contributor Author

Keyrxng commented Sep 14, 2023

2/ The assignee should still be Keyrxng.

This feels sort of wrong, you wanna at least split it @wannacfuture?

@wannacfuture
Copy link
Contributor

wannacfuture commented Sep 14, 2023

2/ The assignee should still be Keyrxng.

This feels sort of wrong, you wanna at least split it @wannacfuture?

Just take it. @Keyrxng. I can think as hotfix as the responsibility for core team member. no problem.

@Keyrxng
Copy link
Contributor Author

Keyrxng commented Sep 14, 2023

Humbled mate, I appreciate it

@0xcodercrane
Copy link
Contributor

0xcodercrane commented Sep 14, 2023

Thank you guys for the great swarming lol

Please re-request a review from me once done. @Keyrxng

Copy link
Contributor

@0xcodercrane 0xcodercrane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would work. merging

@0xcodercrane 0xcodercrane merged commit 53fc6ab into ubiquity:development Sep 18, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reopened & Unassigned
4 participants