Unstack Updates: preflight PR eligibility before delete and improve API errors#136
Unstack Updates: preflight PR eligibility before delete and improve API errors#136wiseemily88 wants to merge 3 commits into
Conversation
…s.Block unstack delete only when all PRs in the stack are ineligible
There was a problem hiding this comment.
Pull request overview
This PR enhances the unstack command's safety by adding a preflight check that prevents stack deletion when all associated pull requests are ineligible (merged, queued for merge, or have auto-merge enabled). It also adds handling for HTTP 422 errors from the GitHub API during stack deletion, providing clearer feedback to the user. The changes are well-tested with four new test cases covering the blocking, mixed eligibility, API failure, and 422 error scenarios.
Changes:
- Added
shouldBlockUnstackDeletepreflight function that checks PR eligibility before attempting to delete a stack on GitHub, blocking only when all PRs are ineligible - Added specific handling for HTTP 422 responses from
DeleteStack, surfacing the API error message - Updated existing tests and added new test cases covering preflight blocking, mixed eligibility, API lookup failures, and 422 error handling
Show a summary per file
| File | Description |
|---|---|
| cmd/unstack.go | Added shouldBlockUnstackDelete function with PR eligibility logic, integrated preflight call before DeleteStack, and added 422 HTTP error handling |
| cmd/unstack_test.go | Added four new test cases for preflight scenarios and updated existing tests to include PullRequestRef fields to accommodate the new preflight check |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
skarim
left a comment
skarim
left a comment
There was a problem hiding this comment.
One minor comment, otherwise looks good! 💯
Co-authored-by: Sameen Karim <skarim@github.com>
francisfuzz
left a comment
francisfuzz
left a comment
There was a problem hiding this comment.
✅ !
To confirm: we'll need https://github.com/github/github/pull/436056 to land first, before merging this in -- 👍🏽 ?
part of https://github.com/github/pull-requests/issues/24519
related to api changes here: https://github.com/github/github/pull/436056/overview
This pull request enhances the safety and robustness of the
unstackcommand by adding a preflight check to prevent unstacking when all associated pull requests (PRs) are ineligible (e.g., merged, queued for merge, or have auto-merge enabled). It also improves error handling and test coverage for these scenarios. Includes enhanced error messages for failed preflight checks and for specific HTTP errors (such as 422) returned by the GitHub API during stack deletion, providing clearer feedback to the user.