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

feat(eval): support coffeescript #230

Merged
merged 9 commits into from
Apr 17, 2021
Merged

feat(eval): support coffeescript #230

merged 9 commits into from
Apr 17, 2021

Conversation

Anillc
Copy link
Member

@Anillc Anillc commented Apr 14, 2021

No description provided.

@codecov
Copy link

codecov bot commented Apr 14, 2021

Codecov Report

Merging #230 (2731758) into develop (51552f7) will decrease coverage by 0.00%.
The diff coverage is 89.65%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #230      +/-   ##
===========================================
- Coverage    92.38%   92.37%   -0.01%     
===========================================
  Files           56       57       +1     
  Lines        10794    10823      +29     
  Branches      2215     2223       +8     
===========================================
+ Hits          9972     9998      +26     
- Misses         822      825       +3     
Impacted Files Coverage Δ
packages/plugin-eval/src/worker/loader.ts 91.13% <57.14%> (-1.07%) ⬇️
packages/plugin-eval/src/loaders/coffeescript.ts 100.00% <100.00%> (ø)
packages/koishi-core/src/help.ts 99.15% <0.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51552f7...2731758. Read the comment docs.

Copy link
Member

@shigma shigma left a comment

Choose a reason for hiding this comment

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

加个测试样例吧,fixture 里面写段 coffee

packages/plugin-eval/src/loaders/coffeescript.ts Outdated Show resolved Hide resolved
Copy link
Member

@shigma shigma left a comment

Choose a reason for hiding this comment

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

extractScript 我之后补个单测再说

packages/plugin-eval/src/loaders/coffeescript.ts Outdated Show resolved Hide resolved
@shigma
Copy link
Member

shigma commented Apr 15, 2021

已更新代码。

  1. 按照你的要求调整的 moduleLoader 后缀名的处理逻辑,请麻烦将 coffee 加在 ts 后面,json 前面。
  2. 添加了 scriptLoader 单元测试,请补充 coffeescript 的部分。

辛苦了。

@Anillc
Copy link
Member Author

Anillc commented Apr 15, 2021

是否需要merge一下并解决冲突

@shigma
Copy link
Member

shigma commented Apr 15, 2021

y

Copy link
Member

@shigma shigma left a comment

Choose a reason for hiding this comment

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

jsx 属于 irrelevant feature,不应该被写进这个 pr

@Anillc Anillc requested a review from shigma April 16, 2021 15:32
Copy link
Member

@shigma shigma left a comment

Choose a reason for hiding this comment

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

LGTM

@shigma shigma merged commit 61e5ea9 into koishijs:develop Apr 17, 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

2 participants