-
Notifications
You must be signed in to change notification settings - Fork 61
fix: get all commits from pr #723
base: development
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for ubiquibot-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Please add rate limiter checker like here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just took a quick look and if you look at the PR #649 30th commit date is August 19 4AM UTC. But if you look at the follow up comment the last activity date is Aug 18 2023 05:57:07 which is exactly the same time as the last comment on the issue. While this does solve the issue with commits pagination, it looks like the bot is looking only at the comments but not the PR. Can you research further?
I guess this issue is related to drafted pull request. @pavlovcik was right. There were 2 problems related to this issue. 1 is that not all commits are fetched and another one is drafted pull requests are filtered to not be considered according to https://github.com/ubiquity/ubiquibot/blob/e9cfeccac40928132b32bebbce564dd9b93e9091/src/helpers/issue.ts#L636C17-L636C17 |
Could you have a look on this please, @whilefoo ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because you changed the function to return draft PRs too, you should be careful about other functions using this function. I think getAvailableOpenedPullRequests
should still filter draft PRs so can you fix that?
Thanks @whilefoo |
Can you post a QA example please. Just set the follow up time to something very short and open two pull requests to two issues. One as a draft and one as a ready pull request. |
I am not sure how to QA this pr. May we need to wait for few days? |
Just set the follow up time to something very short in your config. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you post QA?
Yes, I will |
@ByteBallet any updates? |
Sorry for delaying... |
@ByteBallet please post QA |
Could you please check the error I mentioned? I am sick recently so didn't have chance to work on it. :( So please allow me some time if this issue is not urgent... |
Codebase looks good. We need a QA to merge the PR @ByteBallet. |
Yes, I know it but I have issues while running the bot. The bot says "the command is disabled in this repo." although I set all the configs to true. I am not so proficient in blockchain and dapp. Is it something related to it? |
If you are facing that kinda issue you can just slash the command disable config check feature out so that it will accept the commands even the config is set false. But before that Plaes make sure if all of them are set correctly for your repo like Readme.md. |
If you @ByteBallet still have an issue with your netlify account, you can also do a QA by following up the QA section in Readme.md. |
Resolves #653
Just changed to fetch all the commits from pr.
By default it listCommits only fetches 30 commits from pr so couldn't get last commit.