-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Allow user select eap version of JetBrains IDE #7852
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,7 @@ const scheme = { | |
"nodes": { "type": "array", "items": { "type": "string" } }, | ||
"hidden": { "type": "boolean" }, | ||
"image": { "type": "string" }, | ||
"latestImage": { "type": "string" }, | ||
"resolveImageDigest": { "type": "boolean" }, | ||
}, | ||
"required": [ | ||
|
@@ -190,6 +191,20 @@ export class IDEConfigService { | |
} | ||
} | ||
|
||
for (const [id, option] of Object.entries(newValue.ideOptions.options).filter(([_, x]) => x.latestImage)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this for loop is fairly complex, with many things happening in a single line - did you consider breaking it down a bit? |
||
try { | ||
value.ideOptions.options[id].latestImage = await this.resolveImageDigest(option.latestImage!); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: I would find it a bit "cleaner" to use a boolean like Here, we seem to use However, not blocking the review due to this. Please feel free to address in a follow-up PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. latestImage is a string not boolean, so There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @iQQBot! Yes, I understand this, but my point was, I prefer something like this: type Option = {
resolveLatestImage: boolean;
latestImage?: string;
}
if (option.resolveLatestImage) {
option.latestImage = await resolve();
} where the controlling condition is different from what's actually resolved. Currently, the code seems to work like this: type Option = {
latestImage?: string;
}
if (option.latestImage) {
option.latestImage = await resolve();
} This makes it a bit unclear if an image has already been resolved or not, e.g.:
|
||
log.info("ide config: successfully resolved latest image digest", { | ||
ide: id, | ||
latestImage: option.latestImage, | ||
resolvedImage: value.ideOptions.options[id].latestImage, | ||
trigger, | ||
}); | ||
} catch (e) { | ||
log.error('ide config: error while resolving latest image digest', e, { trigger }); | ||
} | ||
} | ||
|
||
const key = JSON.stringify(value); | ||
if (key === this.state.key) { | ||
return; | ||
|
Uh oh!
There was an error while loading. Please reload this page.