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.

Sign up for GitHub

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

Jump to bottom

Add commit statuses reports on pull request view #6845

Merged
merged 24 commits into from
Jun 30, 2019
Merged

Add commit statuses reports on pull request view #6845

merged 24 commits into from
Jun 30, 2019

Conversation

lunny
Copy link
Member

@lunny lunny commented May 4, 2019 •
edited
Loading

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

default theme:

image

arc-green theme:

image

@lunny lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. 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
lafriks
lafriks reviewed May 5, 2019
View reviewed changes
models/migrations/v85.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 5, 2019
@strk
Copy link
Member

strk commented May 5, 2019

is multiple statuses per commit something supported already ?

@lunny
Copy link
Member Author

lunny commented May 5, 2019

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

@lunny lunny added this to the 1.9.0 milestone May 20, 2019
@lunny lunny force-pushed the lunny/status_reports branch 3 times, most recently from 93fdaf4 to e71aadd Compare May 26, 2019 12:30
@lunny lunny force-pushed the lunny/status_reports branch 2 times, most recently from cc4e4dd to 63d53bf Compare June 6, 2019 01:10
@lunny lunny mentioned this pull request Jun 8, 2019
Closed
7 tasks
@lunny lunny force-pushed the lunny/status_reports branch 2 times, most recently from 4a75955 to e057fd6 Compare June 10, 2019 02:20
@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
Copy link
Member Author

lunny commented Jun 10, 2019

It's ready to review

@codecov-io
Copy link

codecov-io commented Jun 10, 2019 •
edited
Loading

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.

jonasfranz
jonasfranz suggested changes Jun 10, 2019
View reviewed changes
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 Show resolved Hide resolved
@jonasfranz
Copy link
Member

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

@lunny
Copy link
Member Author

lunny 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
Copy link
Member Author

lunny commented Jun 10, 2019

@jonasfranz done.

jonasfranz
jonasfranz reviewed Jun 11, 2019
View reviewed changes
models/commit_status.go Outdated Show resolved Hide resolved
@lunny
Copy link
Member Author

lunny commented Jun 11, 2019

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

jonasfranz
jonasfranz reviewed Jun 13, 2019
View reviewed changes
models/commit_status.go Outdated Show resolved Hide resolved
@lunny
Copy link
Member Author

lunny 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
Copy link
Member Author

lunny commented Jun 30, 2019

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

@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 Jun 30, 2019
@zeripath zeripath merged commit ff85dd3 into go-gitea:master Jun 30, 2019
@lunny lunny deleted the lunny/status_reports branch June 30, 2019 08:01
@lafriks lafriks added the type/changelog Adds the changelog for a new Gitea version label Jun 30, 2019
@lunny lunny mentioned this pull request Jul 1, 2019
Merged
jeffliu27 pushed a commit to jeffliu27/gitea that referenced this pull request Jul 18, 2019
@lunny @jeffliu27
Add commit statuses reports on pull request view (go-gitea#6845)
7f87b63 
* 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
Copy link
Contributor

@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
Copy link
Member Author

lunny commented Aug 1, 2019

That's next PR of mine, see #7481

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
@delvh delvh removed the type/changelog Adds the changelog for a new Gitea version label Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dr-consit dr-consit dr-consit left review comments

@filipnavara filipnavara filipnavara left review comments

@lafriks lafriks lafriks left review comments

@techknowlogick techknowlogick techknowlogick left review comments

@jonasfranz jonasfranz jonasfranz approved these changes

@zeripath zeripath zeripath approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
1.9.0
Development

Successfully merging this pull request may close these issues.