Pull DetailView: Show more pull informations #271

Merged

6543 merged 7 commits from 6543/tea:enhance-pullDetailView into master 2 years ago

6543 commented 2 years ago

Collaborator

close #258

6543 added 1 commit 2 years ago

3467819ce7

Pull Detailview: add head/base-branch, reviews, mergable info

6543 added the

kind/enhancement

label 2 years ago

6543 added this to the v0.6.0 milestone 2 years ago

6543 added 1 commit 2 years ago

113c826f85 Merge branch 'master' into enhance-pullDetailView

6543 commented 2 years ago

Poster

Collaborator

demo:

Bildschirmfoto zu 2020-12-06 23-47-54.png

58 KiB

khmarbaise reviewed 2 years ago

Dismissed

cmd/pulls.go Outdated

khmarbaise commented 2 years ago

Collaborator

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?

6543 commented 2 years ago

Poster

Collaborator

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

khmarbaise commented 2 years ago

Collaborator

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

noerw commented 2 years ago

Collaborator

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

6543 commented 2 years ago

Poster

Collaborator

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

noerw marked this conversation as resolved

noerw reviewed 2 years ago

Dismissed

modules/print/pull.go

noerw commented 2 years ago

Collaborator

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