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

blue-tape -> node-tap #2223

Merged
merged 13 commits into from Aug 14, 2021
Merged

blue-tape -> node-tap #2223

merged 13 commits into from Aug 14, 2021

Conversation

bung87
Copy link
Contributor

@bung87 bung87 commented Jul 29, 2021

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
  • CI has been passed. (GitHub actions all turns green)
  • CLA has been signed

Description

blue-tape is deprecated.

@CLAassistant
Copy link

CLAassistant commented Jul 29, 2021

CLA assistant check
All committers have signed the CLA.

@huan
Copy link
Member

huan commented Aug 12, 2021

Thank you for the contrinbution.

However, I can not understand it, so could you please explain why you submit this PR?

What's the problem with the current version of code, and what solution you are trying to make?

@bung87
Copy link
Contributor Author

bung87 commented Aug 12, 2021

blue-tape is deprecated, and it relys on tape, instead of use tape with node-tap, I think only use node-tap is fine.

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.

LGTM if the blue-tap has been deprecated.

Please help me to understand you by replying to my two comments, resolve the conflicts, and then I believe we can continue to review them.

Thanks!

tsconfig.json Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
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.

Thanks for following my review comments!

There are some more issues need to be fixed in this PR, please fix them before we can continue reviewing it.

.eslintrc.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/user/room.spec.ts Outdated Show resolved Hide resolved
tests/.eslintrc.js Outdated Show resolved Hide resolved
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.

Thanks for the updates!

I noticed that we have void before tests and I have reproduced the problems if we remove them.

We need to find a better way to avoid adding void in our tests.

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.

LGTM

Thank you very much for all the great works for converting the Wechaty testing framework from blue-tape to the latest version of tap!

It seems that we still have some issues about the tap and also need some improvement, but this PR can be merged for now.

Appreciate it!

@huan huan merged commit 110cd1c into wechaty:master Aug 14, 2021
@bung87
Copy link
Contributor Author

bung87 commented Aug 14, 2021

Thanks for the updates!

I noticed that we have void before tests and I have reproduced the problems if we remove them.

We need to find a better way to avoid adding void in our tests.

I haven't found a better solution to resolve the problem , as these test use callback style with async function, if there's one that requires more code change I think.

@huan
Copy link
Member

huan commented Aug 14, 2021

Thank you very much for your contribution!

You are welcome to join Wechaty Contributor Program

1. Join Wechaty Organization

You've invited Bung to Wechaty! They'll be receiving an email shortly. They can also visit https://github.com/wechaty to accept the invitation.

I have invited you to join our Wechaty GitHub Organization, please accept it by following the above message. (See also: wechaty/PMC#16)

2. Update Your Wechaty Contributor Profile

  1. Please open Contributor Hall of Fame and add yourself to the end of the list, so that other contributors will know you better!
  2. Please add yourself to our Website Contributors by creating a PR and refer to this PR link as well.

3. Join The Contributor Only WeChat Room

We also have a WeChat room for contributors only which can discuss Wechaty at a deeper level, you are welcome to join and if you are interested.

Please add @lijiarui wechat: ruirui_0914 and send her this pr link. She will invite you into Wechaty Contributor Room

Cheers!

huan added a commit to huan/tstest that referenced this pull request Aug 15, 2021
huan added a commit that referenced this pull request Aug 15, 2021
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.

None yet

3 participants