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: get all commits from pr #723

Open
wants to merge 16 commits into
base: development
Choose a base branch
from

Conversation

kamaalsultan
Copy link
Contributor

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.

@netlify
Copy link

netlify bot commented Sep 5, 2023

Deploy Preview for ubiquibot-staging ready!

Name Link
🔨 Latest commit bc6406c
🔍 Latest deploy log https://app.netlify.com/sites/ubiquibot-staging/deploys/651cc3b75d97b00008c2f1eb
😎 Deploy Preview https://deploy-preview-723--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.

Copy link
Collaborator

@whilefoo whilefoo left a 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

Copy link
Collaborator

@whilefoo whilefoo left a 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?

@kamaalsultan
Copy link
Contributor Author

kamaalsultan commented Sep 11, 2023

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
I am not sure how to QA this pr

@kamaalsultan
Copy link
Contributor Author

Could you have a look on this please, @whilefoo ?

Copy link
Collaborator

@whilefoo whilefoo left a 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?

@kamaalsultan
Copy link
Contributor Author

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
I added a state parameter to the function so we can get "ready for review" or "draft" or all.

@0x4007
Copy link
Member

0x4007 commented Sep 13, 2023

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.

@kamaalsultan
Copy link
Contributor Author

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?

@0x4007
Copy link
Member

0x4007 commented Sep 13, 2023

Just set the follow up time to something very short in your config.

Copy link
Collaborator

@whilefoo whilefoo left a 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?

@kamaalsultan
Copy link
Contributor Author

can you post QA?

Yes, I will
I am contacting with Netlify support team because I have some issues with my Netlify account. I will post QA asap.

@whilefoo
Copy link
Collaborator

whilefoo commented Sep 22, 2023

@ByteBallet any updates?

@kamaalsultan
Copy link
Contributor Author

kamaalsultan commented Sep 22, 2023

@ByteBallet any updates?

Sorry for delaying...
I deployed the on Netlify and wanted to conduct QA but some issues are occuring :( byteballets/ubiquibot_qa#1 (comment)

.github/ubiquibot-config.yml Outdated Show resolved Hide resolved
src/configs/shared.ts Outdated Show resolved Hide resolved
@whilefoo
Copy link
Collaborator

@ByteBallet please post QA

@kamaalsultan
Copy link
Contributor Author

kamaalsultan commented Sep 26, 2023

@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...

src/helpers/issue.ts Outdated Show resolved Hide resolved
src/helpers/issue.ts Outdated Show resolved Hide resolved
src/helpers/issue.ts Outdated Show resolved Hide resolved
@0xcodercrane
Copy link
Contributor

Codebase looks good. We need a QA to merge the PR @ByteBallet.

@kamaalsultan
Copy link
Contributor Author

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?

@wannacfuture
Copy link
Contributor

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.

@0xcodercrane
Copy link
Contributor

0xcodercrane commented Oct 5, 2023

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.

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.

Draft Pull Request not Recognized as Hunter Activity
5 participants