-
Notifications
You must be signed in to change notification settings - Fork 35
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 #59: when editing an exisiting project, add base starter if no dependencies are selected. #62
Conversation
@@ -61,7 +61,7 @@ export class DependencyManager { | |||
} | |||
|
|||
public getSelectedDependencies(): IDependency[] { | |||
return this.selectedIds.map((id: string) => this.dict[id]); | |||
return this.selectedIds.map((id: string) => this.dict[id]).filter(Boolean); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for robustness
@@ -26,7 +27,7 @@ export namespace dependencies { | |||
const rawJSONString: string = await Utils.downloadFile(`${Utils.settings.getServiceUrl()}dependencies?bootVersion=${bootVersion}`, true, { Accept: "application/vnd.initializr.v2.1+json" }); | |||
starters[bootVersion] = JSON.parse(rawJSONString); | |||
} | |||
return starters[bootVersion]; | |||
return _.cloneDeep(starters[bootVersion]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
always return a copy, in case the caller modify the cached content.
@@ -186,8 +186,8 @@ export module Routines { | |||
vscode.window.showInformationMessage("No changes."); | |||
return; | |||
} | |||
const msgRemove: string = (toRemove && toRemove.length) ? `Removing: [${toRemove.map(d => manager.dict[d].name).join(", ")}].` : ""; | |||
const msgAdd: string = (toAdd && toAdd.length) ? `Adding: [${toAdd.map(d => manager.dict[d].name).join(", ")}].` : ""; | |||
const msgRemove: string = (toRemove && toRemove.length) ? `Removing: [${toRemove.map(d => manager.dict[d] && manager.dict[d].name).filter(Boolean).join(", ")}].` : ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avoid potential NPE
@@ -196,6 +196,14 @@ export module Routines { | |||
TelemetryHelper.finishStep(stepProceed); | |||
} | |||
|
|||
// add spring-boot-starter if no selected starters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add default starter, same issue with spring-projects/sts4#52
The title can be edited to mention that this is about editing an existing project. start.spring.io will always give you the base starter if no dependencies are selected. |
@snicoll changed as the suggestion. |
When generating a project from Spring Initializr, it always gives you the base starter if no dependencies are selected.
So when we remove all the dependencies during editing, it should add the base starter back.