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 support for monorepos and JS-Controller 5 #371

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

AlCalzone
Copy link
Contributor

@AlCalzone AlCalzone commented Feb 16, 2023

With this PR and ioBroker/ioBroker.js-controller#2103, dev-server can now be used in the JS-Controller 5 monorepo. The install command must be changed slightly, because dev-server needs to know where the "entrypoint" is, and we should be using symlinks to avoid having to pack and install 8 packages on each change:

dev-server setup --entrypoint packages/controller --symlinks

For executing the controller, it is recommended NOT to build and not to install (symlinks make the later unnecessary). Not sure if we should automatically do this:

dev-server run/debug/watch -x

TODO:

  • dev-server run
  • dev-server debug
  • dev-server watch
  • make the call to buildAdapter before installing configurable
  • fix TODOs in code
  • npmignore checking fails
  • Windows support

@AlCalzone AlCalzone marked this pull request as ready for review March 28, 2023 19:48
@AlCalzone
Copy link
Contributor Author

This has been tested extensively and seems to be working, with the caveat that the dev has to manually execute npm run watch:ts for the controller in a separate terminal.

backupFile: string | undefined,
useSymlinks: boolean,
): Promise<void> {
await this.buildLocalAdapter();
// await this.buildLocalAdapter();
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: personally I am not a fan of out commented code, as we have git history to know how it looked like a while ago

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I may have forgotten to clean this up 😇

Copy link
Collaborator

@foxriver76 foxriver76 left a comment

Choose a reason for hiding this comment

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

on a quick overview it lgtm

The open todos are still open? If you found out already, you may adapt them.

@Apollon77
Copy link
Collaborator

@dependabot rebase

@AlCalzone
Copy link
Contributor Author

@Apollon77 I'm not Dependabot 🫣

@Apollon77
Copy link
Collaborator

lol :-) sorry

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.

3 participants