JJ / github-pr-contains-action

Action that checks whether the body or diff in a PR contains a certain word.
MIT License
29 stars 33 forks source link

Make action compatible with private repos. #113

Closed ApoorvGuptaAi closed 8 months ago

ApoorvGuptaAi commented 8 months ago

Make action compatible with private repos.

[x] Unit tests run and pass.

ApoorvGuptaAi commented 8 months ago

Example run on private repo:

Screenshot 2023-11-06 at 9 18 02 PM
JJ commented 8 months ago

Make action compatible with private repos.

* Switches to the latest github actions package and API.

* Use octokit rest API interface to get PR diff (works for private repos too).

Wow, amazing. Thanks a lot. I need some time to check it out, even to understand it properly. Stick around, don't go far, and again, thanks a lot.

ApoorvGuptaAi commented 8 months ago

Wow, amazing. Thanks a lot. I need some time to check it out, even to understand it properly. Stick around, don't go far, and again, thanks a lot.

Couple of things to look out for:

JJ commented 8 months ago

Wow, amazing. Thanks a lot. I need some time to check it out, even to understand it properly. Stick around, don't go far, and again, thanks a lot.

Couple of things to look out for:

* Havent tested the waivedUser functionality, specifically if non-"User" type users should be checked.

That was one of my questions, probably something should be conveyed to the user in that case, but so far I didn't know such kind of users existed... Might be worth the while a separate PR, maybe, although what is there so far looks great.

* Tested on small PRs on private repos, I dont know if the new diff API has any limitations on size

In diff size, you mean? At any rate, maybe add a caveat in the documentation?

JJ commented 8 months ago

Also, waivedUsers is not working. I have to get around to it since summer #106

ApoorvGuptaAi commented 8 months ago

Have responded to and fixed everything.

@JJ Havent tested the latest set of changes, is there some way to test it out?

JJ commented 8 months ago

Just the basics, I just approved running them. I don't have time ATM but I would say all looks good... Thanks!

ApoorvGuptaAi commented 8 months ago

Just confirmed itsworking, please go ahead and merge.