Skip to content
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

Edit room doc #1570

Merged
merged 16 commits into from
Aug 27, 2018
Merged

Edit room doc #1570

merged 16 commits into from
Aug 27, 2018

Conversation

kis87988
Copy link
Contributor

@kis87988 kis87988 commented Aug 27, 2018

I'm submitting added documentation

[ ] Bug Fix
[ ] Feature
[X] Other (Refactoring, Added tests, Documentation, ...)

Checklist

  • Commit Messages follow the Conventional Commits pattern
    • A feature commit message is prefixed "feat:"
    • A bugfix commit message is prefixed "fix:"
  • Tests for the changes have been added

Description

Edit and add some documentation in room.ts and API list in README.md

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Copy link
Member

@huan huan left a comment

Choose a reason for hiding this comment

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

Hi! It's great to see this PR for filling the API list documentation, really appreciate it.

I had made a comment on the PR. Could you please follow my review and made the necessary change before I could be able to merge your PR?

Thank you very much!

src/user/room.ts Outdated
* @returns {Promise<Contact[]>}
* @example
* const room_list:Conatct[]|null = await room.findAll()
* if(room_list)
Copy link
Member

Choose a reason for hiding this comment

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

Could you please follow the naming style from the JS community instead of a python style?

Please change room_list to roomList, and so do others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok 😄

src/user/room.ts Outdated
* console.log("room all member list: ", room_list)
* const member_contact_list: Conatct[]|null =await room.findAll("abc")
* const memberContactList: Conatct[] | null =await room.findAll("abc")
Copy link
Member

Choose a reason for hiding this comment

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

"abc" to 'abc' please, because all Wechaty source code is using '

I'll merge this PR after you fix this, thanks!

src/user/room.ts Outdated
* @example
* const roomList:Conatct[] | null = await room.findAll()
* if(roomList)
* console.log("room all member list: ", room_list)
Copy link
Member

Choose a reason for hiding this comment

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

"abc" to 'abc' please, because all Wechaty source code is using '

I'll merge this PR after you fix this, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

src/user/room.ts Outdated
@@ -875,7 +875,7 @@ export class Room extends Accessory implements Sayable {
* const roomList:Conatct[] | null = await room.findAll()
* if(roomList)
* console.log("room all member list: ", room_list)
* const memberContactList: Conatct[] | null =await room.findAll("abc")
* const memberContactList: Conatct[] | null =await room.findAll('abc')
* console.log("contact list with all name, room alias, alias are abc:", member_contact_list)
Copy link
Member

Choose a reason for hiding this comment

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

There's more double quotas, please fix them all.

src/user/room.ts Outdated
* @example
* const roomList:Conatct[] | null = await room.findAll()
* if(roomList)
* console.log("room all member list: ", room_list)
Copy link
Member

Choose a reason for hiding this comment

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

here's also a double quota

@huan
Copy link
Member

huan commented Aug 27, 2018

Awesome, thank you very much for the contribution!

@huan huan merged commit 86e2287 into wechaty:master Aug 27, 2018
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.

2 participants