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

fix: refactor workers into their own package #6382

Merged
merged 11 commits into from
Mar 13, 2021
Merged

Conversation

benbrown
Copy link
Contributor

@benbrown benbrown commented Mar 11, 2021

Description

refactors the server workers into their own package.

cuts direct dependencies on server internals.

this allows the workers to be unpacked in the electron repo and used.

Task Item

fixes #6372

@cwhitten cwhitten added the 1.4 label Mar 11, 2021
@benbrown benbrown marked this pull request as ready for review March 11, 2021 23:51
@boydc2014
Copy link
Contributor

@lei9444 can you help take a look at this one, and verify if there is impact to the way other workers are used or packed?

@@ -8,23 +8,22 @@ import yeoman from 'yeoman-environment';
import TerminalAdapter from 'yeoman-environment/lib/adapter';
Copy link
Contributor

Choose a reason for hiding this comment

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

remove commented code
a bunch of awaits down below are not necessary, linter shows them.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hatpick my recent change upgrade the yeoman-environment package, but the @types package has not been updated upstream- so the linter may warn about unnecessary awaits but the methods being invoked do return promises.

Composer/packages/server-workers/src/dialogMerge.worker.ts Outdated Show resolved Hide resolved
Composer/packages/server/src/models/asset/assetManager.ts Outdated Show resolved Hide resolved
Composer/packages/server/src/utility/project.ts Outdated Show resolved Hide resolved
Composer/packages/server-workers/package.json Outdated Show resolved Hide resolved
"dependencies": {
"yeoman-environment": "^2.10.3",
"format-message": "^6.2.3",
"@microsoft/bf-dialog": "4.11.0-dev.20201025.69cf2b9"
Copy link
Member

Choose a reason for hiding this comment

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

should this be 4.12.x?

Composer/packages/server/src/models/asset/assetManager.ts Outdated Show resolved Hide resolved
Composer/packages/server/src/utility/project.ts Outdated Show resolved Hide resolved
@a-b-r-o-w-n
Copy link
Contributor

@benbrown and I discussed offline, I am going to finish up this work today.

the @types package is outdated, so we'll want to update that when a new version is published
@coveralls
Copy link

coveralls commented Mar 12, 2021

Coverage Status

Coverage increased (+0.09%) to 52.982% when pulling 90cbee3 on benbrown/workermagic into 727bd53 on main.

@cwhitten cwhitten merged commit 230ac9a into main Mar 13, 2021
@cwhitten cwhitten deleted the benbrown/workermagic branch March 13, 2021 00:57
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
* refactor workers into their own package

* unpack the new server-workers module
exclude server-workers deps from yarn workspace

* adjustments to workspace settings

* fix typo in path

* expose ServerWorker class for workers

* update yeoman-environment

the @types package is outdated, so we'll want to update that when a new version is published

* mock server worker in test

Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Co-authored-by: Andy Brown <asbrown002@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't create bot with component model flag on
6 participants