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

feat: change module.ts to runtime.ts #3323

Merged
merged 3 commits into from
Jul 7, 2020
Merged

feat: change module.ts to runtime.ts #3323

merged 3 commits into from
Jul 7, 2020

Conversation

sspku-yqLiu
Copy link
Contributor

issue: #3286

@CLAassistant
Copy link

CLAassistant commented Jul 7, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@imsobear
Copy link
Collaborator

imsobear commented Jul 7, 2020

插件开发的文档可以顺手改下

@chenbin92 chenbin92 self-requested a review July 7, 2020 03:54
@chenbin92
Copy link
Collaborator

chenbin92 commented Jul 7, 2020 •
edited
Loading

其他插件下的 module.tsx 也需要改下

image

@chenbin92
Copy link
Collaborator

image

PR commit 需要规范下，描述具体做了什么事情

chenbin92
chenbin92 reviewed Jul 7, 2020
View reviewed changes
packages/plugin-core/src/index.ts Outdated
const modulePath = path.join(path.dirname(pluginPath), 'module.js');
// TODO: module.js will be discarded in future.
let modulePath = path.join(path.dirname(pluginPath), 'runtime.js');
modulePath = fse.existsSync(modulePath) ? modulePath : path.join(path.dirname(pluginPath), 'module.js');
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里应该是 TODO -> NOTE。然后可以打印一个 WARN 提示

@sspku-yqLiu
Copy link
Contributor Author

刚看到那个TODO改为note 我修改一下

@chenbin92 chenbin92 merged commit 710e756 into alibaba:master Jul 7, 2020
chenbin92 pushed a commit that referenced this pull request Jul 7, 2020
Revert "feat: change module.ts to runtime.ts (#3323)"
ac66db5 
This reverts commit 710e756.
@chenbin92 chenbin92 mentioned this pull request Jul 7, 2020
Merged
@weekly-digest weekly-digest bot mentioned this pull request Jul 12, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chenbin92 chenbin92 chenbin92 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.
4 participants
@sspku-yqLiu @CLAassistant @imsobear @chenbin92