Skip to content

Conversation

@helloocc
Copy link
Contributor

  1. query room ignore case
  2. coverage decrease will cause CI failed, now off.
@helloocc helloocc requested a review from a team as a code owner June 28, 2022 15:53
if not payload:
return False
if query == payload.id or query in payload.topic:
if query == payload.id or (query.lower() in payload.topic.lower()):
Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hard to fix static check:)

Comment on lines 29 to 30
project: off
patch: off
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you give more desc for this changes, so that I can easily understand what you have done ? and I notice that the latest version of coverage is V3, can you upgrade it ? If so, it will be more great for this contribution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I notice that if coverage decrease, x will be show in commit. like this: 6940577 So, turn off is better.
163317103-b3c3510d-8f5a-4ab3-bf27-a8da93a99e81
However, I will not change it this patch.

Copy link
Collaborator

@wj-Mcat wj-Mcat left a comment

Choose a reason for hiding this comment

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

Your contribution is great, and there are some things need to be discussed.

@helloocc helloocc force-pushed the master branch 2 times, most recently from 6979352 to abe1edd Compare July 7, 2022 05:00
@helloocc helloocc changed the title Query room ignore case, coverage project/patch off. Query room ignore case, mention text show contact name. Jul 9, 2022
Copy link
Collaborator

@wj-Mcat wj-Mcat left a comment

Choose a reason for hiding this comment

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

LGTM

@wj-Mcat wj-Mcat merged commit 4bc72f0 into wechaty:master Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants