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 missing commit states to PR checks template #11085

Merged
merged 6 commits into from Apr 16, 2020

Conversation

CirnoT
Copy link
Contributor

@CirnoT CirnoT commented Apr 15, 2020

The template was missing case for failure state, causing 'Some checks are pending' to be displayed incorrectly.

There was also a missing case for warning - I have decided to use translation for success in such case, but we might want to add new translation for it.


On an unrelated note, and possibly for another PR; we might want to distinguish between error and failure state in translations, just like GitHub does (errored vs failed) as they can mean very different things (for example CI failed to execute pipeline due to some internal error vs pipeline failed)

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 15, 2020
@CirnoT
Copy link
Contributor Author

CirnoT commented Apr 16, 2020

Added translation string for warning as per review.

Took liberty of using this opportunity to separate failure and error as well, since they are differentiated by icons everywhere else.

@CirnoT CirnoT requested a review from 6543 April 16, 2020 01:57
@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 Apr 16, 2020
@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 16, 2020
@lafriks lafriks added the type/enhancement An improvement of existing functionality label Apr 16, 2020
@lafriks lafriks added this to the 1.12.0 milestone Apr 16, 2020
@jolheiser
Copy link
Member

Ping lgtm

@jolheiser jolheiser merged commit e938266 into go-gitea:master Apr 16, 2020
@lunny
Copy link
Member

lunny commented Apr 17, 2020

Shouldn't this be backport to v1.11 ?

@lafriks
Copy link
Member

lafriks commented Apr 18, 2020

It is just display issue, not critical bug

@CirnoT CirnoT deleted the patch-3 branch April 21, 2020 15:34
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
* Add missing commit states to PR checks template

* Add separate translation strings for warning and error

* Fix failure status string

* Revert accidental change with whitespace
@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.

None yet

6 participants