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

Allow emoji on review head comments #10159

Merged
merged 2 commits into from
Feb 6, 2020
Merged

Allow emoji on review head comments #10159

merged 2 commits into from
Feb 6, 2020

Conversation

guillep2k
Copy link
Member

@guillep2k guillep2k commented Feb 6, 2020 •
edited
Loading

Fixes #9895

There are other kinds of comment that would probably benefit from the has-emoji class but I'm not sure which ones. Here's some candidates, I accept suggestions:

  • CommentTypeCommitRef 4
  • CommentTypeLock 23
  • CommentTypeUnlock 24

And on the titles of other issues in add/remove dependency, maybe.

image

@lunny lunny added the type/bug label Feb 6, 2020
@lunny lunny added this to the 1.12.0 milestone Feb 6, 2020
@lunny lunny added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Feb 6, 2020
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Feb 6, 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 Feb 6, 2020
@silverwind
Copy link
Member

silverwind commented Feb 6, 2020 •
edited
Loading

There are other kinds of comment that would probably benefit

What's the reason for this whitelist-based approach via has-emoji anways? Why not run the replacer on the whole document with a reasonable blacklist instead?

@sapk
Copy link
Member

sapk commented Feb 6, 2020

I added backport as It feel it would be an easy pick to backport.

@lafriks lafriks merged commit 08c9160 into go-gitea:master Feb 6, 2020
@lafriks
Copy link
Member

lafriks commented Feb 6, 2020

Please send backport

@guillep2k guillep2k deleted the fix-9895 branch February 7, 2020 10:50
guillep2k added a commit to guillep2k/gitea that referenced this pull request Feb 7, 2020
@guillep2k @techknowlogick
Allow emoji on review head comments (go-gitea#10159)
190e660 
Co-authored-by: techknowlogick <matti@mdranta.net>
@guillep2k guillep2k mentioned this pull request Feb 7, 2020
Merged
@sapk sapk added the backport/done All backports for this PR have been created label Feb 7, 2020
sapk added a commit that referenced this pull request Feb 7, 2020
@guillep2k @techknowlogick @sapk
Allow emoji on review head comments (#10159) (#10174)
b5b4436 
Co-authored-by: techknowlogick <matti@mdranta.net>

Co-authored-by: techknowlogick <matti@mdranta.net>
Co-authored-by: Antoine GIRARD <sapk@users.noreply.github.com>
@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.
Reviewers

@lunny lunny lunny approved these changes

@6543 6543 6543 approved these changes

@jolheiser jolheiser jolheiser approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.

Emoji in review comments not rendering
9 participants
@guillep2k @silverwind @sapk @lafriks @lunny @6543 @jolheiser @GiteaBot @techknowlogick