Display commit status on landing page of repo #1784
Conversation
|
To follow the merged #1688, this could be done before adding the information for PR. |
tboerger
added
the
lgtm/need 2
|
CI failed. |
|
I saw, I need to rebase on master so the build will not failed since RepoCreationNum as been renamed in MaxCreationLimit. |
DblK
force-pushed
the
feature_DisplayCommitStatusOnRepo
branch
from
fc466a5 to
15024de
Compare
routers/repo/view.go
Outdated
| @@ -119,6 +120,21 @@ func renderDirectory(ctx *context.Context, treeLink string) { | |||
| ctx.Data["LatestCommit"] = latestCommit | |||
| ctx.Data["LatestCommitVerification"] = models.ParseCommitWithSignature(latestCommit) | |||
| ctx.Data["LatestCommitUser"] = models.ValidateCommitWithEmail(latestCommit) | |||
| commit := models.SignCommitWithStatuses{ | |||
There was a problem hiding this comment.
Is there a reason you don't just use two local variables? The SignCommit field of commit never comes into play, so I think that just using variables would be simpler.
There was a problem hiding this comment.
Good point. I took some portion of code from the display. I will use local variable instead.
templates/repo/view_list.tmpl
Outdated
| @@ -26,6 +26,23 @@ | |||
| {{end}} | |||
| </a> | |||
| <span class="grey has-emoji">{{RenderCommitMessage false .LatestCommit.Summary .RepoLink $.Repository.ComposeMetas}}</span> | |||
| {{if .LatestCommitStatus}} | |||
There was a problem hiding this comment.
Can this code be moved to separate template and reused in both places?
routers/repo/view.go
Outdated
| State: "", | ||
| Statuses: make([]*models.CommitStatus, 0), | ||
| } | ||
| commit.Statuses, err = models.GetLatestCommitStatus(ctx.Repo.Repository, ctx.Repo.Commit.ID.String(), 0) |
There was a problem hiding this comment.
Why not get the CommitStatus for the latest SHA? (As-is now you're introducting race-conditions)
There was a problem hiding this comment.
Disregard this, I missed the Commit.ID.String()-part 🙄
There was a problem hiding this comment.
This is duplicated code from ParseCommitsWithStatus function.
There was a problem hiding this comment.
Since ctx.Repo.Commit has parsed and not used ParseCommitsWithStatus, I will simple handle here and maybe another PR to do further improvement.
|
@DblK Any update? 🙂 |
|
Kind of busy right now, I'll work on it later. |
|
@DblK Any updates on this? |
|
@DblK if you are some busy, could some maintainers help you to finish this PR? |
|
If some maintainers want to finish then go for it. |
lunny
force-pushed
the
feature_DisplayCommitStatusOnRepo
branch
from
15024de to
3a59b61
Compare
|
See below three places status and all have links to CI now.
|
|
LGTM |
tboerger
added
lgtm/need 1
|
LGTM |
tboerger
added
lgtm/done
|
@lunny please rebase |
Codecov Report
@@ Coverage Diff @@
## master #1784 +/- ##
==========================================
- Coverage 27.85% 27.83% -0.02%
==========================================
Files 83 83
Lines 16825 16835 +10
==========================================
Hits 4686 4686
- Misses 11464 11474 +10
Partials 675 675
Continue to review full report at Codecov.
|
lunny
force-pushed
the
feature_DisplayCommitStatusOnRepo
branch
from
f87655e to
2c706c4
Compare
|
@lafriks done. |
|
@ethantkoenig oh sorry, just found need your approval after I merged. |
|
@lunny No worries, it looks my suggestion was addressed nonetheless |
|
@ethantkoenig OK. |
This will implement a part of #996 and so #1029.
The modification of the ui will be like this:
Status is now displayed.