Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show last commit status in pull request lists #6465

Merged
merged 5 commits into from
Apr 2, 2019

Conversation

yzzyx
Copy link
Contributor

@yzzyx yzzyx commented Mar 29, 2019

Show last commit status for pull requests in lists.
Addresses part of issue #996 (Better CI Integration).

Takes a sligthly different approach than PR #2519 (Add commit status on repo and user pull request lists) to minimize the changes needed to implement the feature.
Screenshot_2019-03-28 test
Screenshot_2019-03-28 Gitea Git with a cup of tea

@lunny lunny added the type/enhancement An improvement of existing functionality label Mar 29, 2019
@lunny lunny added this to the 1.9.0 milestone Mar 29, 2019
@codecov-io
Copy link

codecov-io commented Mar 29, 2019

Codecov Report

Merging #6465 into master will increase coverage by 0.09%.
The diff coverage is 70.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6465      +/-   ##
==========================================
+ Coverage   40.17%   40.26%   +0.09%     
==========================================
  Files         402      402              
  Lines       54020    54051      +31     
==========================================
+ Hits        21703    21765      +62     
+ Misses      29309    29275      -34     
- Partials     3008     3011       +3
Impacted Files Coverage Δ
routers/repo/issue.go 36.8% <100%> (+0.38%) ⬆️
routers/user/home.go 36.93% <100%> (+0.76%) ⬆️
models/pull.go 50.55% <52.63%> (+0.03%) ⬆️
routers/repo/view.go 42.07% <0%> (+0.99%) ⬆️
models/repo_list.go 67.89% <0%> (+1.05%) ⬆️
modules/git/repo_commit.go 47.63% <0%> (+1.45%) ⬆️
modules/log/event.go 65.98% <0%> (+1.52%) ⬆️
models/status.go 72.98% <0%> (+2.29%) ⬆️
routers/repo/pull.go 36.67% <0%> (+2.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 704da08...730fc75. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 29, 2019
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 29, 2019
@jolheiser
Copy link
Member

jolheiser commented Mar 29, 2019

It seems this doesn't make a distinction for multiple PRs on one repo.

All of the below PR have a commit status assigned to the latest commit in the respective PR, however in the lists only the most recently updated PR shows the commit status.
latest_commit

EDIT: Doing more testing, and now it seems to be showing some correctly....not sure if I had bad data or if something else was happening...

I made new commit statuses along with a new PR, but it doesn't seem to be showing.
example1
example2

EDIT 2: It seems to be missing the two most recent PRs, and showing the rest.
missing_two

Sorry for the long comment, hopefully it helps track it down. Or let me know if I'm doing something wrong.

@yzzyx
Copy link
Contributor Author

yzzyx commented Mar 30, 2019

@jolheiser:
Can you see any commits in the 'commit_status' table for those last two branches?
Nothing will be shown if there is no commit status available (no CI integration, or CI integration has not reported any status yet).

I made a simple test myself, but I can't seem to reproduce your view, unless I stop jenkins from receiving updates.
What steps did you perform?

Below is a screenshot that were created by adding a simple jenkinsfile, creating three different branches, and then one PR for each of those.
Screenshot_2019-03-30 Gitea Git with a cup of tea(1)

@jolheiser
Copy link
Member

I will test with a fresh db.
I tested using the API directly.

Copy link
Member

@jolheiser jolheiser left a comment

Choose a reason for hiding this comment

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

On a fresh DB this worked great! (I must have had some bad testing data in my old test DB)

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 1, 2019
routers/repo/issue.go Show resolved Hide resolved
@techknowlogick techknowlogick merged commit bf5af87 into go-gitea:master Apr 2, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants