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

[WIP] Improve CommitStatus #26247

Closed
wants to merge 10 commits into from
Closed

Conversation

yp05327
Copy link
Contributor

@yp05327 yp05327 commented Jul 31, 2023

Mentioned in #25990 (comment)
Fix #25990

Changes:

  1. Using Int to save CommitStatus in DB
  2. Using min in sql to get the CommitStatus instead of calculate CommitStatus in Gitea side
  3. Adding a pager bar in web ui

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 31, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 31, 2023
@yp05327 yp05327 marked this pull request as draft July 31, 2023 08:55
{{svg "gitea-exclamation" 18 "commit-status icon text red"}}
{{end}}
{{if eq .State "failure"}}
{{if eq .State 2}}
Copy link
Member

Choose a reason for hiding this comment

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

I believe that using direct numbers for judgment here leads to poor code readability and maintainability. My opinion is to continue using the previous code on the frontend page.When passing the state to the frontend, perform a conversion from numbers to strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, forgot that we have CommitStatusState.IsXXXX.
I think this is the best solution.

}

// NoBetterThan returns true if this State is no better than the given State
// This function only handles the states defined in CommitStatusPriorities
func (css CommitStatusState) NoBetterThan(css2 CommitStatusState) bool {
Copy link
Member

Choose a reason for hiding this comment

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

This requires checking for special values, enhancing the robustness of the function, and avoiding issues caused by incorrect usage by the caller, as seen in #26121.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finally, I want to remove this function.
It is only used in MergeRequiredContextsCommitStatus. But have no good idea about how to rewrite it now.
Or maybe I will add CommitStatusState.IsValid which should be called before we compare them.

@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 1, 2023
silverwind added a commit that referenced this pull request Nov 2, 2023
Step one for a GitHub like commit status check ui:

![image](https://github.com/go-gitea/gitea/assets/18380374/22953b88-1f91-4d19-bc57-ad92d33fa11f)

![image](https://github.com/go-gitea/gitea/assets/18380374/78572a49-c9b0-472b-86a8-8293197e807b)

![image](https://github.com/go-gitea/gitea/assets/18380374/bc5c8d1c-2ab5-4b03-b8c6-20c34b86d856)

Step two:

![image](https://github.com/go-gitea/gitea/assets/18380374/938b359e-8823-4192-b82d-55fa40b986fd)

![image](https://github.com/go-gitea/gitea/assets/18380374/2de5bb8f-40f5-462a-8d6d-bac13a32bc2a)

The design now will list all commit status checks which takes too much
space.
This is a pre-improve for #26247

---------

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
fuxiaohei pushed a commit to fuxiaohei/gitea that referenced this pull request Jan 17, 2024
)

Step one for a GitHub like commit status check ui:

![image](https://github.com/go-gitea/gitea/assets/18380374/22953b88-1f91-4d19-bc57-ad92d33fa11f)

![image](https://github.com/go-gitea/gitea/assets/18380374/78572a49-c9b0-472b-86a8-8293197e807b)

![image](https://github.com/go-gitea/gitea/assets/18380374/bc5c8d1c-2ab5-4b03-b8c6-20c34b86d856)

Step two:

![image](https://github.com/go-gitea/gitea/assets/18380374/938b359e-8823-4192-b82d-55fa40b986fd)

![image](https://github.com/go-gitea/gitea/assets/18380374/2de5bb8f-40f5-462a-8d6d-bac13a32bc2a)

The design now will list all commit status checks which takes too much
space.
This is a pre-improve for go-gitea#26247

---------

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
silverwind added a commit to silverwind/gitea that referenced this pull request Feb 20, 2024
)

Step one for a GitHub like commit status check ui:

![image](https://github.com/go-gitea/gitea/assets/18380374/22953b88-1f91-4d19-bc57-ad92d33fa11f)

![image](https://github.com/go-gitea/gitea/assets/18380374/78572a49-c9b0-472b-86a8-8293197e807b)

![image](https://github.com/go-gitea/gitea/assets/18380374/bc5c8d1c-2ab5-4b03-b8c6-20c34b86d856)

Step two:

![image](https://github.com/go-gitea/gitea/assets/18380374/938b359e-8823-4192-b82d-55fa40b986fd)

![image](https://github.com/go-gitea/gitea/assets/18380374/2de5bb8f-40f5-462a-8d6d-bac13a32bc2a)

The design now will list all commit status checks which takes too much
space.
This is a pre-improve for go-gitea#26247

---------

Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@yp05327 yp05327 closed this Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pull request check list is limited to 30 entries
3 participants