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

Add commit statuses reports on pull request view #6845

Merged
merged 24 commits into from Jun 30, 2019

Conversation

@lunny
Copy link
Member

commented May 4, 2019

This pull request will add commit statuses report on pull request view page.

default theme:

image

arc-green theme:

image

@lunny lunny added the kind/feature label May 4, 2019

@lunny lunny changed the title WIP: Add commit statuses reports on pull view WIP: Add commit statuses reports on pull request view May 4, 2019

@lunny lunny force-pushed the lunny:lunny/status_reports branch from d0a9a4d to 68ada3f May 4, 2019

Show resolved Hide resolved models/migrations/v85.go Outdated

@GiteaBot GiteaBot added the lgtm/need 2 label May 5, 2019

@strk

This comment has been minimized.

Copy link
Member

commented May 5, 2019

is multiple statuses per commit something supported already ?

@lunny

This comment has been minimized.

Copy link
Member Author

commented May 5, 2019

@strk yes, it was supported when commit status added.

@lunny lunny force-pushed the lunny:lunny/status_reports branch from a190995 to f59d6ba May 20, 2019

@lunny lunny added this to the 1.9.0 milestone May 20, 2019

@lunny lunny force-pushed the lunny:lunny/status_reports branch 3 times, most recently from 93fdaf4 to e71aadd May 21, 2019

@lunny lunny force-pushed the lunny:lunny/status_reports branch 2 times, most recently from cc4e4dd to 63d53bf Jun 1, 2019

@lunny lunny referenced this pull request Jun 8, 2019

Closed

Commit status indicator doesn't show latest status #7134

2 of 7 tasks complete

@lunny lunny force-pushed the lunny:lunny/status_reports branch 2 times, most recently from 4a75955 to e057fd6 Jun 9, 2019

@lunny lunny changed the title WIP: Add commit statuses reports on pull request view Add commit statuses reports on pull request view Jun 10, 2019

@lunny

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2019

It's ready to review

@codecov-io

This comment has been minimized.

Copy link

commented Jun 10, 2019

Codecov Report

Merging #6845 into master will decrease coverage by 0.01%.
The diff coverage is 39.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6845      +/-   ##
==========================================
- Coverage   41.25%   41.24%   -0.02%     
==========================================
  Files         464      466       +2     
  Lines       63107    63153      +46     
==========================================
+ Hits        26036    26045       +9     
- Misses      33666    33702      +36     
- Partials     3405     3406       +1
Impacted Files Coverage Δ
models/migrations/migrations.go 1.52% <ø> (ø) ⬆️
models/migrations/v87.go 0% <ø> (ø) ⬆️
models/migrations/v88.go 0% <0%> (ø)
routers/api/v1/repo/status.go 21.02% <25%> (-0.09%) ⬇️
modules/repofiles/commit_status.go 55% <55%> (ø)
routers/repo/pull.go 31.7% <60%> (+0.65%) ⬆️
models/commit_status.go 77.02% <70.37%> (ø)
models/gpg_key.go 55.83% <0%> (-0.84%) ⬇️
... and 2 more

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 1e46eed...18bb02e. Read the comment docs.

Show resolved Hide resolved models/commit_status.go Outdated
Show resolved Hide resolved modules/repofiles/commit_status.go Outdated
Show resolved Hide resolved options/locale/locale_en-US.ini Outdated
@jonasfranz

This comment has been minimized.

Copy link
Member

commented Jun 10, 2019

@lunny Can you explain for what the StatusHash is needed?

@lunny

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2019

@jonasfranz In an old version, I want to save avatar/logo to table commit_status_context and after that I decided to split it as two parts. That's why we need a commit_status_context table. And secondly, as an index, TEXT will result in some problem, so I added status_context_hash as index.

@lunny

This comment has been minimized.

Copy link
Member Author

commented Jun 10, 2019

@jonasfranz done.

Show resolved Hide resolved models/commit_status.go Outdated
@lunny

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

@jonasfranz I think you are right. The new table is unnecessary and I have removed it.

Show resolved Hide resolved models/commit_status.go Outdated

@lunny lunny force-pushed the lunny:lunny/status_reports branch from 4079d3f to e007595 Jun 18, 2019

@lunny

This comment has been minimized.

Copy link
Member Author

commented Jun 20, 2019

So, maybe we should have three tables:

table 1: commit_status (exsited): store commit status, will drop context column and store context id.
table 2: commit_status_contexts (add): store contexts, context(hash) will be unique
table 3: commit_status_repo_context (add): unique with repo_id and context_id, so that we can find all repo's context easier.

lunny added some commits May 26, 2019

@lunny lunny force-pushed the lunny:lunny/status_reports branch from aa616ad to 4ab23d4 Jun 30, 2019

@lunny

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2019

@jonasfranz @zeripath done. Please see my updated screenshot on top comment.

@GiteaBot GiteaBot added lgtm/done and removed lgtm/need 1 labels Jun 30, 2019

@zeripath zeripath merged commit ff85dd3 into go-gitea:master Jun 30, 2019

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr Build is passing
Details

@lunny lunny deleted the lunny:lunny/status_reports branch Jun 30, 2019

@lafriks lafriks added the changelog label Jun 30, 2019

stertingen added a commit to stertingen/gitea that referenced this pull request Jul 2, 2019

Add commit statuses reports on pull request view (go-gitea#6845)
* Add commit statuses reports on pull view

* Add some translations

* improve the UI

* fix fmt

* fix tests

* add a new test git repo to fix tests

* fix bug when headRepo or headBranch missing

* fix tests

* fix tests

* fix consistency

* fix tests

* fix tests

* change the test repo

* fix tests

* fix tests

* fix migration

* keep db size consistency

* fix translation

* change commit hash status table unique index

* remove unused table

* use char instead varchar

* make hashCommitStatusContext private

* split merge section with status check on pull view ui

* fix tests; fix arc-green theme on pull ui

jeffliu27 added a commit to jeffliu27/gitea that referenced this pull request Jul 18, 2019

Add commit statuses reports on pull request view (go-gitea#6845)
* Add commit statuses reports on pull view

* Add some translations

* improve the UI

* fix fmt

* fix tests

* add a new test git repo to fix tests

* fix bug when headRepo or headBranch missing

* fix tests

* fix tests

* fix consistency

* fix tests

* fix tests

* change the test repo

* fix tests

* fix tests

* fix migration

* keep db size consistency

* fix translation

* change commit hash status table unique index

* remove unused table

* use char instead varchar

* make hashCommitStatusContext private

* split merge section with status check on pull view ui

* fix tests; fix arc-green theme on pull ui
@rakshith-ravi

This comment has been minimized.

Copy link

commented Aug 1, 2019

@lunny how do I enable a branch to be merged only after status checks pass? I'm trying this on my 1.9.0 instance but I'm not able to see any button that says "Enable status checks" or something similar

@lunny

This comment has been minimized.

Copy link
Member Author

commented Aug 1, 2019

That's next PR of mine, see #7481

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.