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: add built-in service #471

Merged
merged 27 commits into from
Oct 21, 2021
Merged

feat: add built-in service #471

merged 27 commits into from
Oct 21, 2021

Conversation

mortalYoung
Copy link
Collaborator

@mortalYoung mortalYoung commented Oct 13, 2021

简介

  • 新增 built-in service ,支持禁用内置模块
  • activityBar 支持 sortIndex

主要变更

  • 取消各个模块的默认值,默认值统一通过该模块对应的 controller 在 loadExtension 之后进行默认值的赋值
  • 由于默认值的赋值是在 loadExtension 之后,则势必出现顺序问题,故为个别模块添加必要的 sortIndex 属性

@mortalYoung mortalYoung added enhancement New feature or request feature labels Oct 13, 2021
@mortalYoung mortalYoung self-assigned this Oct 13, 2021
@codecov
Copy link

codecov bot commented Oct 14, 2021

Codecov Report

Merging #471 (b9f28f3) into main (33558b3) will increase coverage by 3.28%.
The diff coverage is 87.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #471      +/-   ##
==========================================
+ Coverage   81.81%   85.10%   +3.28%     
==========================================
  Files         178      179       +1     
  Lines        5109     5183      +74     
  Branches     1110     1178      +68     
==========================================
+ Hits         4180     4411     +231     
+ Misses        920      763     -157     
  Partials        9        9              
Impacted Files Coverage Δ
src/controller/layout.ts 87.50% <0.00%> (+16.07%) ⬆️
src/controller/sidebar.ts 83.33% <0.00%> (+3.33%) ⬆️
src/extensions/activityBar/index.ts 27.27% <0.00%> (-12.73%) ⬇️
src/extensions/index.ts 100.00% <ø> (ø)
src/extensions/panel/index.ts 25.00% <ø> (-15.00%) ⬇️
src/model/keybinding.ts 100.00% <ø> (ø)
src/model/settings.ts 100.00% <ø> (ø)
src/model/workbench/activityBar.ts 100.00% <ø> (ø)
src/model/workbench/explorer/editorTree.ts 0.00% <ø> (-80.00%) ⬇️
src/model/workbench/menuBar.ts 100.00% <ø> (ø)
... and 76 more

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 33558b3...b9f28f3. Read the comment docs.

@mortalYoung
Copy link
Collaborator Author

覆盖率达大降的原因是因为,我取消了部分 controller 的 resolve 的时机,而在 provider 中 resolve 的话,我是通过 modules.forEach 实现的,覆盖率检测不到 0.0 所以我需要一个一个 controller 补充测试用例

@mortalYoung
Copy link
Collaborator Author

覆盖率达大降的原因是因为,我取消了部分 controller 的 resolve 的时机,而在 provider 中 resolve 的话,我是通过 modules.forEach 实现的,覆盖率检测不到 0.0 所以我需要一个一个 controller 补充测试用例

终于过了 🤣

@mortalYoung mortalYoung changed the title WIP: feat: add built-in service feat: add built-in service Oct 20, 2021
@mortalYoung mortalYoung requested a review from wewoor October 20, 2021 10:50
@mortalYoung mortalYoung changed the title feat: add built-in service [WIP]: feat: add built-in service Oct 20, 2021
@@ -53,6 +54,7 @@ export class FolderTreeController

const fileContextMenu = this.folderTreeService.getFileContextMenu();
const folderContextMenu = this.folderTreeService.getFolderContextMenu();
const { ROOT_FOLDER_CONTEXT_MENU } = this.builtinService.getModules();
Copy link
Collaborator

Choose a reason for hiding this comment

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

我们可以把 modules 和 constants 的命名区分下,不然会看起来比较奇怪,这里会怀疑获取的是一个不同的 Constant

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

确实,命名是这个 PR 里面直接改了吗,还是说另起一个 PR 改?我个人倾向于另起一个 0.0

Copy link
Collaborator

@wewoor wewoor Oct 21, 2021

Choose a reason for hiding this comment

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

重启一个吧,我重开一个 issue #475

src/controller/explorer/folderTree.tsx Show resolved Hide resolved
@mortalYoung mortalYoung changed the title [WIP]: feat: add built-in service feat: add built-in service Oct 21, 2021
@wewoor wewoor merged commit d96e2b3 into main Oct 21, 2021
@wewoor wewoor deleted the feat/builtin branch October 21, 2021 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants