search

sopel-irc / sopel-github

GitHub plugin for Sopel
Other
3 stars 13 forks source link

formatting: use `emoji` 2.0 style #119

Closed ofalk closed 1 year ago

ofalk commented 1 year ago

use language='alias' instead of use_aliases=True, which was deprecated in 1.7.0, see also: https://github.com/carpedm20/emoji/blob/v1.7.0/emoji/core.py

Pretty straight forward change. Let me know if there is anything else that I can/should do, since I already have the fork open.

dgw commented 1 year ago

Indeed, it is straightforward to do it this way. I suggest updating the README to document that emoji 2.0 or higher is now required for this to work, and update the commit message so the actual change is shown in the top line.

Commit message example:

formatting: use `emoji` 2.0 style

The `use_aliases` argument was deprecated in `emoji` 1.7 and removed
in 2.0. It was replaced with the option `language='alias'`.

Fixes #118. See also:
https://github.com/carpedm20/emoji/blob/d8bbfe455c6fcd12b96ed1dce6e0978fe7a47431/emoji/core.py#L95-L98

(We do obey the 72-character wrapping rule, except for URLs, because we want them to stay clickable.)

ofalk commented 1 year ago

Hey! Makes sense! But I cannot commit to do this before Friday, if that's fine?

dgw commented 1 year ago

No big rush! As I said before, the workaround is easy (downgrade to emoji<2).

ofalk commented 1 year ago

There we go. Updated the commit message.

dgw commented 1 year ago

Readme update?

ofalk commented 1 year ago

Crap. I knew there was something I missed. 😂 I'll update the readme in the next days!

ofalk commented 1 year ago

Alrighty. What about this?

dgw commented 1 year ago

Rather than add a new section, I was expecting an update to the existing mention here: https://github.com/sopel-irc/sopel-github/blob/588db54d477a46bc6e8db277869a1a4240318412/README.md?plain=1#L47-L49

And if you want to give an install command with >= specified, it has to be quoted: pip install 'emoji>=2.0' (otherwise pip will install just emoji, and Bash will send its output to a file called =2.0).

Realistically though, I wouldn't worry too much about getting the readme perfect. That feature really should be a setuptools extra, with the requirement specified in package metadata. This is largely why I had in mind a minimal update to the existing mention of using the emoji package, because it's not going to stay this way for very long.

ofalk commented 1 year ago

OK, we're getting there! Update README again.

dgw commented 1 year ago

Hmm, bad force-push? image

ofalk commented 1 year ago

Seems, I missed to amed the commit :-(

ofalk commented 1 year ago

Thanks a lot @dgw ! Esp. for your patience to get this done.