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.

Sign up for GitHub

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

Jump to bottom

Drop @types/mocha dependency #1104

Merged
merged 4 commits into from
Dec 20, 2024
Merged

Drop @types/mocha dependency #1104

merged 4 commits into from
Dec 20, 2024

Conversation

Yang-33
Copy link
Contributor

@Yang-33 Yang-33 commented Dec 19, 2024

We haven't used mocha, so @types/mocha dependency is not necessary for now.
(follow-up for #793)

@Yang-33 Yang-33 marked this pull request as ready for review December 19, 2024 01:02
@Yang-33 Yang-33 requested a review from a team December 19, 2024 01:02
@Yang-33 Yang-33 enabled auto-merge (squash) December 19, 2024 01:02
@Yang-33 Yang-33 disabled auto-merge December 19, 2024 01:21
habara-k
habara-k reviewed Dec 19, 2024
View reviewed changes
Copy link
Contributor

@habara-k habara-k left a comment

Choose a reason for hiding this comment

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

How about removing mocha from under examples/ as well?

@Yang-33
Copy link
Contributor Author

Yang-33 commented Dec 19, 2024

It doesn't seem all package.json in examples specify mocha. Could you elaborate on that? We should not modify package-lock.json manually.

@Yang-33 Yang-33 requested a review from a team December 19, 2024 02:04
@habara-k
Copy link
Contributor

line-bot-sdk-nodejs/examples/rich-menu/package.json

Line 11 in 20bb1c3

"@line/bot-sdk": "../../"
depends on the root package.

For example, running npm install in examples/rich-menu removes the dependency on mocha in examples/rich-menu.
It seems like the correct approach is to configure the CI to run npm install for each project under examples.

@Yang-33 Yang-33 force-pushed the bye-type-for-mocha branch 2 times, most recently from 3929c7e to 515a420 Compare December 19, 2024 15:30
@Yang-33
Copy link
Contributor Author

Yang-33 commented Dec 19, 2024

Thank you. The purpose of this PR is solely to remove dependency only for testing that is not used as an SDK.
Is it really necessary to set up CI in this change for the correct approach you said..? I made a PR for this. (#1108) PTAL this PR too.

@Yang-33 Yang-33 requested a review from habara-k December 19, 2024 16:05
@Yang-33 Yang-33 merged commit dad18f9 into line:master Dec 20, 2024
6 checks passed
@Yang-33 Yang-33 deleted the bye-type-for-mocha branch December 20, 2024 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@habara-k habara-k habara-k approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Yang-33 @habara-k