-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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.
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
revise room.say() mention function #1729
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add a unit test for the new Tagged Template modification.
And also please add a description of your BREAKING CHANGE with your PR, so that we could put it into the CHANGELOG later.
BREAKING CHANGE:
room.say(text: string, mention: Contact[]) Alternative solutionTo mention multiple contacts when sending out text messages, there are two new ways of doing that:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job for adding the unit tests.
Please:
- Bump the minor version in package.json file, and add the BREAKING CHANGES to README(in HISTORY section, with the new VERSION).
- Follow my latest comments before we merge this PR.
src/user/room.spec.ts
Outdated
@@ -23,7 +23,7 @@ | |||
import test from 'blue-tape' | |||
import sinon from 'sinon' | |||
|
|||
import { RoomPayload } from 'wechaty-puppet' | |||
import { ContactPayload, RoomMemberPayload, RoomPayload } from 'wechaty-puppet' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import {
ContactPayload,
RoomMemberPayload,
RoomPayload,
} from 'wechaty-puppet'
src/user/room.spec.ts
Outdated
await room.say('hey buddies, let\'s party', contact1, contact2) | ||
await room.say('') | ||
|
||
t.deepEqual(callback.getCall(0).args, [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add subtests for each room.say
test.
i.e.
test('say()', async t => {
// tear up...
test('say() with param1', async t => {
// do say() test 1
})
test('say() with param2', async t => {
// do say() test 2
})
// other subtests ...
})
/** | ||
* 4. Link Message | ||
*/ | ||
await this.puppet.messageSendUrl({ | ||
contactId : this.id | ||
}, textOrContactOrFileOrUrl.payload) | ||
}, textOrListOrContactOrFileOrUrl.payload) | ||
} else if (textOrListOrContactOrFileOrUrl instanceof Array) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a new private function for the following codes for sayTemplateStringsArray()
.
} else if (textOrListOrContactOrFileOrUrl instanceof Array) {
return sayTemplateStringsArray(
textOrListOrContactOrFileOrUrl,
...mentionList,
)
}
@huan Done. |
Thanks for your great PR! |
I'm submitting a...
Refer to #1718
This PR involves a BREAKING CHANGE