Pull DetailView: Show more pull informations #271

Merged
6543 merged 7 commits from 6543/tea:enhance-pullDetailView into master 2020-12-08 04:06:06 +00:00
4 changed files with 52 additions and 9 deletions

View File

@ -5,12 +5,15 @@
package cmd
import (
"fmt"
"code.gitea.io/tea/cmd/flags"
"code.gitea.io/tea/cmd/pulls"
"code.gitea.io/tea/modules/config"
"code.gitea.io/tea/modules/print"
"code.gitea.io/tea/modules/utils"
"code.gitea.io/sdk/gitea"
"github.com/urfave/cli/v2"
)
@ -40,16 +43,22 @@ func runPulls(ctx *cli.Context) error {
func runPullDetail(index string) error {
login, owner, repo := config.InitCommand(flags.GlobalRepoValue, flags.GlobalLoginValue, flags.GlobalRemoteValue)
idx, err := utils.ArgToIndex(index)
if err != nil {
return err
}
pr, _, err := login.Client().GetPullRequest(owner, repo, idx)
client := login.Client()
pr, _, err := client.GetPullRequest(owner, repo, idx)
if err != nil {
return err
}
noerw marked this conversation as resolved Outdated

Just a question:

reviews, _, _ := client.ListPullReviews(owner, repo, idx, gitea.ListPullReviewsOptions{})

This does ignore the returned error from ListPullReviews if I correctly understand that.. so
shouldn't that be changed to populate the error back to the caller? What is here the general
approach in Go? like the following?

reviews, _, err := client.ListPullReviews(owner, repo, idx, gitea.ListPullReviewsOptions{})
if err != nil {
	return err
}

Or do I misunderstand a thing here?

Just a question: ``` reviews, _, _ := client.ListPullReviews(owner, repo, idx, gitea.ListPullReviewsOptions{}) ``` This does ignore the returned error from ListPullReviews if I correctly understand that.. so shouldn't that be changed to populate the error back to the caller? What is here the general approach in Go? like the following? ``` reviews, _, err := client.ListPullReviews(owner, repo, idx, gitea.ListPullReviewsOptions{}) if err != nil { return err } ``` Or do I misunderstand a thing here?
Outdated
Review

yes it just ignore an error ... since this api is realy new and wont work on old instances

yes it just ignore an error ... since this api is realy new and wont work on old instances

Ok. Just wanted to know because my knowledge in Go is very limited. Thanks for your explanation.

Ok. Just wanted to know because my knowledge in Go is very limited. Thanks for your explanation.
Outdated
Review

Maybe put that explanation in a code comment for future reference ;)

Maybe put that explanation in a code comment for future reference ;)
Outdated
Review

It now print a error info in the corner bevore the pull detail view is shown ...

It now print a error info in the corner bevore the pull detail view is shown ...
print.PullDetails(pr)
reviews, _, err := client.ListPullReviews(owner, repo, idx, gitea.ListPullReviewsOptions{})
if err != nil {
fmt.Printf("error while loading reviews: %v\n", err)
}
print.PullDetails(pr, reviews)
return nil
}

View File

@ -143,7 +143,7 @@ func runPullsCreate(ctx *cli.Context) error {
log.Fatalf("could not create PR from %s to %s:%s: %s", head, ownerArg, base, err)
}
print.PullDetails(pr)
print.PullDetails(pr, nil)
fmt.Println(pr.HTMLURL)
return err

View File

@ -13,7 +13,7 @@ import (
// IssueDetails print an issue rendered to stdout
func IssueDetails(issue *gitea.Issue) {
OutputMarkdown(fmt.Sprintf(
"# #%d %s (%s)\n%s created %s\n\n%s\n",
"# #%d %s (%s)\n@%s created %s\n\n%s\n",
issue.Index,
issue.Title,
issue.State,

View File

@ -11,14 +11,48 @@ import (
)
// PullDetails print an pull rendered to stdout
func PullDetails(pr *gitea.PullRequest) {
OutputMarkdown(fmt.Sprintf(
"# #%d %s (%s)\n%s created %s\n\n%s\n",
func PullDetails(pr *gitea.PullRequest, reviews []*gitea.PullReview) {
base := pr.Base.Name
head := pr.Head.Name
if pr.Head.RepoID != pr.Base.RepoID {
if pr.Head.Repository != nil {
head = pr.Head.Repository.Owner.UserName + ":" + head
} else {
head = "delete:" + head
}
}
out := fmt.Sprintf(
"# #%d %s (%s)\n@%s created %s\t**%s** <- **%s**\n\n%s\n",
pr.Index,
pr.Title,
pr.State,
pr.Poster.UserName,
FormatTime(*pr.Created),
base,
head,
pr.Body,
))
)
if len(reviews) != 0 {
out += "\n"
revMap := make(map[string]gitea.ReviewStateType)
for _, review := range reviews {
switch review.State {
case gitea.ReviewStateApproved,
gitea.ReviewStateRequestChanges,
gitea.ReviewStateRequestReview:
revMap[review.Reviewer.UserName] = review.State
}
}
for k, v := range revMap {
out += fmt.Sprintf("\n @%s: %s", k, v)
}
}
if pr.State == gitea.StateOpen && pr.Mergeable {
6543 marked this conversation as resolved
Review

This check should also verify that the minum amount of reviews are approved, to be in line with the web ui (eg this PR currently shows READY TO MERGE.)

Or is pr.Mergeable about git conflicts? In that case you should print No Conflicts instead

This check should also verify that the minum amount of reviews are approved, to be in line with the web ui (eg this PR currently shows `READY TO MERGE`.) Or is `pr.Mergeable` about git conflicts? In that case you should print `No Conflicts` instead
Review

-> https://github.com/go-gitea/gitea/issues/13879

and yes I think it's a good idear to only show NoConflicts at the moment

-> https://github.com/go-gitea/gitea/issues/13879 and yes I think it's a good idear to only show NoConflicts at the moment
out += "\nNo Conflicts"
}
OutputMarkdown(out)
}