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

refactor: app add loader hook #131

Merged
merged 7 commits into from
Jul 27, 2022
Merged

refactor: app add loader hook #131

merged 7 commits into from
Jul 27, 2022

Conversation

JerrysShan
Copy link
Collaborator

No description provided.

@JerrysShan JerrysShan force-pushed the ssk/refactor/loader_hook branch 2 times, most recently from e6eb8e0 to 4522ae7 Compare July 11, 2022 13:06
noahziheng
noahziheng requested changes Jul 12, 2022
View reviewed changes
@JerrysShan JerrysShan force-pushed the ssk/refactor/loader_hook branch from 4522ae7 to 83f154b Compare July 13, 2022 08:55
@JerrysShan
Copy link
Collaborator Author

这个改动主要是为了暴露自定义 loader hook 的API ， 满足上层框架自定义 loader hook 的需求

hyj1991
hyj1991 requested changes Jul 13, 2022
View reviewed changes
@hyj1991
Copy link
Member

hyj1991 commented Jul 13, 2022

LoaderEventEmitter 自身的实现看起来并没有问题

@JerrysShan JerrysShan changed the title WIP: refactor: app add loader hook refactor: app add loader hook Jul 18, 2022
@JerrysShan JerrysShan force-pushed the ssk/refactor/loader_hook branch from b89f4cd to 4aed2b8 Compare July 18, 2022 11:21
@JerrysShan JerrysShan force-pushed the ssk/refactor/loader_hook branch 2 times, most recently from 2181f52 to e59459d Compare July 18, 2022 12:18
@JerrysShan JerrysShan force-pushed the ssk/refactor/loader_hook branch from e59459d to 11bde91 Compare July 18, 2022 12:35
@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2022 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@73e21be). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #131   +/-   ##
=========================================
  Coverage          ?   88.97%           
=========================================
  Files             ?       50           
  Lines             ?     1079           
  Branches          ?      172           
=========================================
  Hits              ?      960           
  Misses            ?      119           
  Partials          ?        0

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 73e21be...478eb27. Read the comment docs.

noahziheng
noahziheng reviewed Jul 25, 2022
View reviewed changes
@noahziheng noahziheng requested a review from hyj1991 July 26, 2022 09:15
@hyj1991
Copy link
Member

hyj1991 commented Jul 27, 2022

style 大量更改最好单独拆 pr，不要合并到 feature 的 pr 里，看起来有点乱

@hyj1991
Copy link
Member

hyj1991 commented Jul 27, 2022

LoaderEventEmitter 自身的实现看起来并没有问题

@JerrysShan
Copy link
Collaborator Author

style 大量更改最好单独拆 pr，不要合并到 feature 的 pr 里，看起来有点乱

我看下，开发完运行了下 lint:fix ，貌似就有 format code 的更改了

@hyj1991
Copy link
Member

hyj1991 commented Jul 27, 2022

style 大量更改最好单独拆 pr，不要合并到 feature 的 pr 里，看起来有点乱

我看下，开发完运行了下 lint:fix ，貌似就有 format code 的更改了

这个 pr 就不用改了，下次可以分开提下

@hyj1991 hyj1991 merged commit 9595ad2 into artusjs:master Jul 27, 2022
@noahziheng noahziheng mentioned this pull request Jul 29, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@noahziheng noahziheng noahziheng approved these changes

@hyj1991 hyj1991 hyj1991 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
@JerrysShan @hyj1991 @codecov-commenter @noahziheng