-
Notifications
You must be signed in to change notification settings - Fork 190
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 project automation logic around closed PRs #396
Conversation
Ooo GraphQL! Interesting! GitHub having two ways to access the API with different levels of features is maddening to no end. Although once you have the PR number, you can check if the PR is open or not using a separate API call and GraphQL isn't required, it's great that we have a cleaner non-hack approach to this. |
The trouble is, I don't think there is any way using the REST API to get the PR number for an associated issue! 🙃 If we could have that then yea we'd be totally set (albeit with more calls), but all we get is this loose connection of "yea this issue has a PR associated with it" and nothing else about said PR 😅 |
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.
Awesome! Thanks for fixing this long pending issue ⭐
Just note that it needs to add pipenv run...
(as it's in the GH workflow) to work, given our current setup. Here is the full command:
ACCESS_TOKEN=<redacted> LOGGING_LEVEL=20 pipenv run python issues_with_prs.py \
--project-number 3 \
--source-column "In progress" \
--target-column "Backlog" \
--linked-pr-state "closed"
Ahh thanks, sorry about that Krystle! I had |
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.
LGTM!
Co-authored-by: Olga Bulat <obulat@gmail.com>
Fixes
Fixes #276 by @krysal
Description
This PR changes the project automation to make it more adaptive to closed PRs. Due to limitations of the GitHub REST API, the previous implementation was only able to check if an issue had an associated PR at all; it was not able to check what state the linked PR was in. The PR information isn't returned in the response either, so there was no way to even match linked PRs with issues using the REST API. This meant that cards with linked, closed PRs were also unconditionally moved. Even if we were to move the stale cards from "In progress" to the "Backlog" column, the next run of the project automation would move them back to "In progress".
Instead, I opted to use the GraphQL API which allows for a bit more configuration. The new query is against PRs (rather than issues), and gathers all PRs in a particular state along with any linked issues. This allows us to only query for open PRs when we want to move something to "In progress", and query for closed PRs when looking to move something back into the "Backlog".
Here's the log output from a local run:
While the details and accuracy of this log are sure to change as we continue working on PRs, it's notable that at the time of writing, all of the cards moved in this case did have closed PRs attached to them.
There's also a potential case where an issue has both closed and open PRs linked to it - in order to handle this appropriately I put the closed PR check before the open PR checks. This means that the card will get moved twice, but it seemed easier than cross referencing each PR. I suppose this has the possibility of clogging up the issue timeline with useless events, that's something we can revisit down the road if we feel it's a problem.
Testing Instructions
This can be tested locally by running
ACCESS_TOKEN=<redacted> LOGGING_LEVEL=20 python issues_with_prs.py --project-number 3 --source-column "In progress" --target-column "Backlog" --linked-pr-state closed
.Checklist
Update index.md
).main
) ora parent feature branch.
errors.
Developer Certificate of Origin
Developer Certificate of Origin