Skip to content

Add gh pr ready#960

Merged
probablycorey merged 11 commits intomasterfrom
prêt-à-réviser
May 19, 2020

Hidden character warning

The head ref may contain hidden characters: "pr\u00eat-\u00e0-r\u00e9viser"
Merged

Add gh pr ready#960
probablycorey merged 11 commits intomasterfrom
prêt-à-réviser

Conversation

@probablycorey
Copy link
Copy Markdown
Contributor

@probablycorey probablycorey commented May 18, 2020 •
edited
Loading

This adds gh pr ready. It looks like this

CleanShot 2020-05-18 at 14 41 00@2x

updated screenshot with @billygriffin's text changes

Closes #740

/cc @ampinsk let me know if you need any design changes

@probablycorey probablycorey requested review from mislav and vilmibm May 18, 2020 20:27
@probablycorey probablycorey self-assigned this May 18, 2020
billygriffin
billygriffin suggested changes May 18, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@billygriffin billygriffin left a comment

Choose a reason for hiding this comment

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

Thanks @probablycorey, looks great overall! I have a couple minor requested changes for verbiage that I think provide a bit more clarity, but happy to discuss.

probablycorey and others added 4 commits May 18, 2020 14:39
@probablycorey @billygriffin
Update command/pr.go
9721f75 
Co-authored-by: Billy Griffin <5091167+billygriffin@users.noreply.github.com>
@probablycorey @billygriffin
Update command/pr.go
6eab375 
Co-authored-by: Billy Griffin <5091167+billygriffin@users.noreply.github.com>
@probablycorey @billygriffin
Update command/pr_test.go
be927b3 
Co-authored-by: Billy Griffin <5091167+billygriffin@users.noreply.github.com>
@probablycorey @billygriffin
Update command/pr_test.go
fde5409 
Co-authored-by: Billy Griffin <5091167+billygriffin@users.noreply.github.com>
probablycorey
probablycorey commented May 18, 2020
View reviewed changes
api/queries_pr.go
Comment on lines +965 to +967
type MarkPullRequestReadyForReviewInput struct {
PullRequestID githubv4.ID `json:"pullRequestId"`
}
Copy link
Copy Markdown
Contributor Author

@probablycorey probablycorey May 18, 2020

Choose a reason for hiding this comment

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

I couldn't find this struct in the githubv4 types, so I made my own. I'm not sure if this is the way to go.

Copy link
Copy Markdown
Contributor

@mislav mislav May 19, 2020

Choose a reason for hiding this comment

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

This looks good! It's most likely a newer API that landed only after the github4 library last synced its schema.

@tierninho
Copy link
Copy Markdown
Contributor

tierninho commented May 19, 2020

Overall, tested well except when I tried inserting the url flag:

➜  cli git:(prêt-à-réviser) ✗ bin/gh pr ready https://github.com/cli/cli/pull/960
no open pull requests found for branch "https://github.com/cli/cli/pull/960"

I expected: "! Pull request #960 is already "ready for review"

mislav
mislav approved these changes May 19, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@mislav mislav left a comment •
edited
Loading

Choose a reason for hiding this comment

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

💯

api/queries_pr.go
Comment on lines +965 to +967
type MarkPullRequestReadyForReviewInput struct {
PullRequestID githubv4.ID `json:"pullRequestId"`
}
Copy link
Copy Markdown
Contributor

@mislav mislav May 19, 2020

Choose a reason for hiding this comment

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

This looks good! It's most likely a newer API that landed only after the github4 library last synced its schema.

@probablycorey
Copy link
Copy Markdown
Contributor Author

probablycorey commented May 19, 2020

Thanks for the catch @tierninho. It should work now with urls

@billygriffin billygriffin self-requested a review May 19, 2020 18:06
billygriffin
billygriffin approved these changes May 19, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@billygriffin billygriffin left a comment

Choose a reason for hiding this comment

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

💥

@tierninho
Copy link
Copy Markdown
Contributor

tierninho commented May 19, 2020

Looks good, thanks for the fix!

@tierninho tierninho self-requested a review May 19, 2020 18:11
@probablycorey probablycorey merged commit e0dbf37 into master May 19, 2020
@probablycorey probablycorey deleted the prêt-à-réviser branch May 19, 2020 18:45
@tranthanhgiang145 tranthanhgiang145 mentioned this pull request Sep 9, 2020
Closed
This was referenced Mar 31, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vilmibm vilmibm Awaiting requested review from vilmibm

3 more reviewers

@mislav mislav mislav approved these changes

@billygriffin billygriffin billygriffin approved these changes

@tierninho tierninho tierninho approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@probablycorey probablycorey

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Ability to mark a draft PR ready for review

4 participants

@probablycorey @tierninho @mislav @billygriffin