Skip to content

Fix broken 👍 emoji#813

Merged
homu merged 1 commit intonew-xkit:masterfrom
Wolvan:bugs/messaging_tweaks/fix_+1_emoji
Dec 23, 2015
Merged

Fix broken 👍 emoji#813
homu merged 1 commit intonew-xkit:masterfrom
Wolvan:bugs/messaging_tweaks/fix_+1_emoji

Conversation

@Wolvan
Copy link
Member

@Wolvan Wolvan commented Dec 1, 2015

The file of the 👍 emoji is called plus1.png instead of +1.png, which
the messaging tweaks has not been taking into account. This is fixed by
simply setting the emoji_url to the proper url instead of the generated
one. Furthermore, the text to be replaced had to be adjusted as well,
since + is a special character in RegEx and does not automatically get
escaped by new RegExp

@invalidCards
Copy link
Member

Is this also the case for 👎 ?

@Wolvan
Copy link
Member Author

Wolvan commented Dec 1, 2015

@ThePsionic 👎 hasn't been broken

@homu
Copy link

homu commented Dec 3, 2015

☔ The latest upstream changes (presumably d35d47b) made this pull request unmergeable. Please resolve the merge conflicts.

@nightpool
Copy link
Member

no idea why my push caused a conflict?

@Wolvan Wolvan force-pushed the bugs/messaging_tweaks/fix_+1_emoji branch from 53b31ed to 15db7c0 Compare December 3, 2015 14:52
@Wolvan
Copy link
Member Author

Wolvan commented Dec 3, 2015

It was a version conflict, but whatever, I rebased so it's fixed

@barosl
Copy link

barosl commented Dec 3, 2015

The merge conflict notification works by detecting the transition from mergeable = false to mergeable = true, so I guess Homu failed to receive the previous event for some reason. It might be due to temporal network problems, or even GitHub itself might be the problem because it does sometimes forget to send an event!

The real cause that made this PR unmergeable seems to be 928b504.

@nightpool
Copy link
Member

yeah, figured it was something like that. Thanks!
On Thu, Dec 3, 2015 at 6:13 PM Barosl Lee notifications@github.com wrote:

The merge conflict notification works by detecting the transition from mergeable
= false to mergeable = true, so I guess Homu failed to receive the
previous event for some reason. It might be due to temporal network
problems, or even GitHub itself might be the problem because it does
sometimes forget to send an event!

The real cause that made this PR unmergeable seems to be 928b504
928b504
.


Reply to this email directly or view it on GitHub
#813 (comment).

@homu
Copy link

homu commented Dec 12, 2015

☔ The latest upstream changes (presumably #864) made this pull request unmergeable. Please resolve the merge conflicts.

The file of the 👍 emoji is called plus1.png instead of +1.png, which
the messaging tweaks has not been taking into account. This is fixed by
simply setting the emoji_url to the proper url instead of the generated
one. Furthermore, the text to be replaced had to be adjusted as well,
since `+` is a special character in RegEx and does not automatically get
escaped by `new RegExp`
@Wolvan Wolvan force-pushed the bugs/messaging_tweaks/fix_+1_emoji branch from 15db7c0 to fa117ac Compare December 12, 2015 21:40
@hobinjk
Copy link

hobinjk commented Dec 23, 2015

Tested locally, 👍
@homu r+

@homu
Copy link

homu commented Dec 23, 2015

📌 Commit fa117ac has been approved by hobinjk

@homu
Copy link

homu commented Dec 23, 2015

⌛ Testing commit fa117ac with merge 5d8c8c3...

homu added a commit that referenced this pull request Dec 23, 2015
…injk

Fix broken 👍 emoji

The file of the 👍 emoji is called plus1.png instead of +1.png, which
the messaging tweaks has not been taking into account. This is fixed by
simply setting the emoji_url to the proper url instead of the generated
one. Furthermore, the text to be replaced had to be adjusted as well,
since `+` is a special character in RegEx and does not automatically get
escaped by `new RegExp`
@homu
Copy link

homu commented Dec 23, 2015

☀️ Test successful - status

@homu homu merged commit fa117ac into new-xkit:master Dec 23, 2015
@homu homu mentioned this pull request Dec 23, 2015
@Wolvan Wolvan deleted the bugs/messaging_tweaks/fix_+1_emoji branch December 24, 2015 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants