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

[plug-in] [vscode] Add some missing entries in env object #4971

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

benoitf
Copy link
Contributor

@benoitf benoitf commented Apr 23, 2019

It's addressing partially #4446 and makes env tests passing at #4224

machineID is using mac address on nodejs while on workers it's a random ID.

Change-Id: I589ede711a18322b1afffe0479dace781f450045
Signed-off-by: Florent Benoit fbenoit@redhat.com

@benoitf benoitf requested a review from evidolob as a code owner April 23, 2019 08:46
@benoitf benoitf requested review from akosyakov and mmorhun April 23, 2019 08:46
@benoitf benoitf self-assigned this Apr 23, 2019
@benoitf benoitf added plug-in system issues related to the plug-in system vscode issues related to VSCode compatibility labels Apr 23, 2019
}

get appRoot() {
return __dirname;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this work in web worker?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will check, I splitted class class for node-mac, I will see how to handle that
good catch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mmorhun : ok I've splitted in two classes, one for worker and one for nodejs

in worker env, we don't provide filesystem path as there is no FS BTW

@@ -20,6 +20,7 @@
"@theia/task": "^0.5.0",
"@theia/workspace": "^0.5.0",
"decompress": "^4.2.0",
"getmac": "^1.4.6",
Copy link
Member

@akosyakov akosyakov Apr 24, 2019

Choose a reason for hiding this comment

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

@marcdumais-work Could you please use your tool to verify this dep?

nvm https://dev.eclipse.org/ipzilla/show_bug.cgi?id=19584 is already resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

Florent and I did the analysis anyway for the new dependency and its own recursive dependency tree. All licenses found are Foundation-approved:

Screenshot from 2019-04-24 10-29-59

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

vscode env tests are green :+1

It's addressing partially #4446

Change-Id: I589ede711a18322b1afffe0479dace781f450045
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
@benoitf benoitf merged commit 4a54abb into master Apr 24, 2019
@benoitf benoitf deleted the env-methods branch April 24, 2019 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plug-in system issues related to the plug-in system vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants