-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
Determine if pr is ready to land #10
Comments
We'll want to make sure this includes "required review" status, and that this reports non-required status checks (but does not consider their status towards the overarching success/failure state), and that if the PR has merge conflicts, it should not be considered ready to land. I don't personally use codeowners, but ideally as well, if a codeowner review is still outstanding, it should not report as ready. |
We could probably add functions to the package https://github.com/sladyn98/hawk-eye-npm that perform something like this:
|
|
I feel that this shouldn't be a separate package, at least for now, since adding functions to a package, publishing it and then updating dependencies in our project doesn't seem ideal since we will have a ton of iterations. I've merged the feature-list branch to main on the MLH fork. I was thinking we can open up a new feature-PR branch and use the code (or more specifically, just the GraphQL query) from sladyn98/hawk-eye-npm instead of the npm package to build on this feature. Need your thoughts @ljharb @sladyn98 @diananova |
@thehanimo Yeah makes sense, I agree with you. I was just playing around with the code and was looking at ways it could be made modular. |
Totally fine to keep in one repo for now; however, my ideal end state is the maximum number of packages at the minimal size/responsibility each :-) |
b) hawk.isMergeable : Checks if a PR has any merge conflicts and whether it can be merged
b) hawk.isPendingReview : Checks if a PR has pending reviews or is approved Usage
Added some more options to the package that will tell us if the pr is mergeable or not and if it has pending review.
However these two solutions would mean we would be taking care of the maintenance and running of these instances/servers and I am not quite sure if that is the right way to go. What do you guys think @thehanimo @ljharb @diananova |
I was assuming polling would be achieved just by looping on the client. |
Yeah that could be done as well :) Simple and efficient |
@ljharb Anything else needed on this issue or can we close? |
The text was updated successfully, but these errors were encountered: