Skip to content

Conversation

@mntlty
Copy link
Collaborator

@mntlty mntlty commented Jul 25, 2022

Fixes #2271

I updated https://github.com/shurcooL/githubv4 to make use of the ConvertPullRequestToDraftInput type.

I originally split this into separate functions, which resulted in a lot of variable passing and largely duplicated code. After a refactor I think this is clear enough as two if statements, as the ready command itself is not too long.

In the issue, there is also an enhancement for gh pr edit, which calls the updatePullRequest graphql api. That api does not support converting a pull request to draft https://docs.github.com/en/graphql/reference/input-objects#updatepullrequestinput
It is possible to use the ConvertPullRequestToDraft function I added in that command as well, I'm not sure how closely the cli and the graphql api should align.

@mntlty mntlty requested a review from a team as a code owner July 25, 2022 19:18
@mntlty mntlty requested review from mislav and removed request for a team July 25, 2022 19:18
@cliAutomation cliAutomation added the external pull request originating outside of the CLI core team label Jul 25, 2022
@mntlty
Copy link
Collaborator Author

mntlty commented Jul 25, 2022

Draft pull requests are not available on all repositories and plans according to https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests#draft-pull-requests

Attempting to convert a pull request to draft results in the following error:

API call failed: Message: mntlty does not have permission to convert the pull request PR_XXXXX to draft., Locations: [{Line:1 Column:76}]

I'm not sure if this is descriptive enough, or if this error should be more informative?

@mntlty
Copy link
Collaborator Author

mntlty commented Aug 1, 2022

@mislav what do you think about this approach?

Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm down with this approach and the messaging, especially since it's called out in the usage that not all plans support the behavior. Will wait to merge until @mislav has had a chance to chime back in, though.

Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great; thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external pull request originating outside of the CLI core team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move PR from ready to Draft

5 participants