Skip to content

fix(chatform): Do not interpret 祝 and 秘 as smiley.#699

Open
nickolay168 wants to merge 1 commit into
TokTok:masterfrom
nickolay168:nickolay168/fix_smileys
Open

fix(chatform): Do not interpret 祝 and 秘 as smiley.#699
nickolay168 wants to merge 1 commit into
TokTok:masterfrom
nickolay168:nickolay168/fix_smileys

Conversation

@nickolay168
Copy link
Copy Markdown

@nickolay168 nickolay168 commented Mar 30, 2026

I have updated the update_smileys.py script so that we can block and unblock smiley without loosing information. I have used the script for removing 祝 and 秘. See #613.


This change is Reviewable

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 30, 2026

Tip

Preview URL:

@github-actions github-actions Bot added the bug Bug fix for the user, not a fix to a build script label Mar 30, 2026
@nickolay168 nickolay168 force-pushed the nickolay168/fix_smileys branch from bcb1695 to 9f47bd8 Compare March 30, 2026 04:48
@nickolay168 nickolay168 force-pushed the nickolay168/fix_smileys branch from 9f47bd8 to 4d27e31 Compare March 30, 2026 05:30
@nickolay168 nickolay168 force-pushed the nickolay168/fix_smileys branch from 4d27e31 to 74d2c16 Compare March 30, 2026 06:07
@nickolay168 nickolay168 force-pushed the nickolay168/fix_smileys branch from 74d2c16 to 43bf715 Compare April 4, 2026 07:58
@nickolay168 nickolay168 force-pushed the nickolay168/fix_smileys branch from 43bf715 to 144b208 Compare April 4, 2026 08:06
@nickolay168 nickolay168 force-pushed the nickolay168/fix_smileys branch from 144b208 to 4e413d3 Compare April 4, 2026 08:23
@Green-Sky
Copy link
Copy Markdown
Member

What is the status of this one?

@nickolay168 nickolay168 force-pushed the nickolay168/fix_smileys branch from 4e413d3 to 942ed73 Compare May 9, 2026 00:34
@nickolay168 nickolay168 force-pushed the nickolay168/fix_smileys branch from 942ed73 to 30b98a1 Compare May 9, 2026 00:57
@nickolay168 nickolay168 force-pushed the nickolay168/fix_smileys branch from 30b98a1 to 8da424e Compare May 9, 2026 01:05
@nickolay168 nickolay168 marked this pull request as ready for review May 9, 2026 01:23
@nickolay168
Copy link
Copy Markdown
Author

What is the status of this one?

I have pushed more changes to fix static analysis errors. I think, it is ready now. The only failing test is related to Bazel issue.

@Green-Sky Green-Sky added this to the v1.18.5 milestone May 9, 2026
Copy link
Copy Markdown
Member

@Green-Sky Green-Sky left a comment

Choose a reason for hiding this comment

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

Looks good otherwise.

Comment thread smileys/update_smileys.py Outdated
@nickolay168
Copy link
Copy Markdown
Author

Looks good otherwise.

I have pushed the fix, but it seems, we have issues with Codacy. Is it configured through UI (It does not seems, we have .codacy.yml)? The linter has both D212 and D213 enabled, which are mutually exclusive. There is also inconsistency in Arguments processing, that results in two errors in codacy.

Comment thread smileys/update_smileys.py Outdated
@Green-Sky
Copy link
Copy Markdown
Member

I have pushed the fix, but it seems, we have issues with Codacy. Is it configured through UI (It does not seems, we have .codacy.yml)? The linter has both D212 and D213 enabled, which are mutually exclusive. There is also inconsistency in Arguments processing, that results in two errors in codacy.

I cant see "D213", but it is supposed to follow https://peps.python.org/pep-0257/#multi-line-docstrings anyway, so lets get it back to that state and then we look at the ci again.

Codacy sadly only shows the last scan for a pr.

The linter in question is Prospector btw.

@nickolay168
Copy link
Copy Markdown
Author

nickolay168 commented May 13, 2026

Prospector

I see. In have ran prospector locally and fixed the issues with Noqa.
By default prospector -D will run both D212 and D213 checks

Multi-line docstring summary should start at the first line (D212)
Multi-line docstring summary should start at the second line (D213)

I have set # noqa D213 comments to allow PEP 257 compliant docstrings.

In some places I have suppressed D407, because if the docstring contains Args:, but rthere is no Return section, it results in failure. I.e. there is no underline under Args: section; section could not contain colon., however

Args
-----
    param: description

will result in error about undocumented parameter "param", because now args is just a section, but not the Arguments description.

@Green-Sky, I have synced the PR could you please take another look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bug fix for the user, not a fix to a build script

Development

Successfully merging this pull request may close these issues.

2 participants