Skip to content

Conversation

@vignesh1992
Copy link
Contributor

Spring init CLI to support both kebab-case and camelCase as project generation options. This PR addresses the issue #26878

Copy link
Member

@snicoll snicoll left a comment

Choose a reason for hiding this comment

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

Thanks for the PR but this adresses only a small part of the original issue. In particular, the issue was about showing consistent options in the CLI:

The code has a mapping from CLI options (kebab-case) to query parameters (camelCase) when issuing a project creation request. I think it needs to map things the other way when displaying information for init --help

The other part was to use kebab case consistently:

We should probably also fix the CLI options so that all multi-word options have a kebab-case variant. Specifically --artifactId and --groupId should be joined by --artifact-id and --group-id respectively. The camelCase variants of the CLI options should be deprecated.

If you intend to only provide a fix for the second part, please let us know and we can then reopen the other issue.

this.name = option(Arrays.asList("name", "n"), "Project name; infer application name").withRequiredArg();
this.description = option("description", "Project description").withRequiredArg();
this.packageName = option("package-name", "Package name").withRequiredArg();
this.packageName = option(Arrays.asList("packageName", "package-name"), "Package name").withRequiredArg();
Copy link
Member

Choose a reason for hiding this comment

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

The plan is to use kebab case consistently, not introducing a mixed case for new properties.

Copy link
Member

Choose a reason for hiding this comment

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

I thought we were going to support both camelCase and kebab-case until we've improved the output of spring init --list so that it converts the server's output from the camelCase that it expects to the kebab-case that the CLI prefers. That improvement to spring init --list was to be handled separately.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, right. I understand now. Yes, so we should have to consistent indeed but we need to deprecate them.

+ "'project' for a project archive)").withRequiredArg().defaultsTo("project");
this.javaVersion = option(Arrays.asList("java-version", "j"), "Language level (for example '1.8')")
.withRequiredArg();
this.javaVersion = option(Arrays.asList("javaVersion", "java-version", "j"),
Copy link
Member

Choose a reason for hiding this comment

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

The plan is to use kebab case consistently, not introducing a mixed case for new properties.

this.language = option(Arrays.asList("language", "l"), "Programming language (for example 'java')")
.withRequiredArg();
this.bootVersion = option(Arrays.asList("boot-version", "b"),
this.bootVersion = option(Arrays.asList("bootVersion", "boot-version", "b"),
Copy link
Member

Choose a reason for hiding this comment

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

The plan is to use kebab case consistently, not introducing a mixed case for new properties.

this.groupId = option(Arrays.asList("groupId", "g"), "Project coordinates (for example 'org.test')")
.withRequiredArg();
this.artifactId = option(Arrays.asList("artifactId", "a"),
this.groupId = option(Arrays.asList("groupId", "group-id", "g"),
Copy link
Member

Choose a reason for hiding this comment

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

groupId is not deprecated.

this.artifactId = option(Arrays.asList("artifactId", "a"),
this.groupId = option(Arrays.asList("groupId", "group-id", "g"),
"Project coordinates (for example 'org.test')").withRequiredArg();
this.artifactId = option(Arrays.asList("artifactId", "artifact-id", "a"),
Copy link
Member

Choose a reason for hiding this comment

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

artifactId is not deprecated.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Oct 1, 2021
@snicoll
Copy link
Member

snicoll commented Oct 1, 2021

@vignesh1992 please ignore my comment about the table. I forgot this is something that should be addressed elsewhere.

@VigneshThangavelIlangovan

Hi @snicoll, Thank you so much for your feedback! Should there be any change which I have to perform or is this PR good to go?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 1, 2021
@philwebb philwebb added for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Oct 8, 2021
@philwebb philwebb added this to the 2.6.x milestone Oct 8, 2021
@philwebb philwebb self-assigned this Oct 21, 2021
@philwebb philwebb modified the milestones: 2.6.x, 2.6.0 Oct 22, 2021
philwebb pushed a commit that referenced this pull request Oct 22, 2021
Update `InitCommand` to support both camelCase and kebab-case.

See gh-28138
philwebb added a commit that referenced this pull request Oct 22, 2021
…ons'

Refine the command so that camelCase options are supported but not
advertised.

See gh-28138
@philwebb philwebb closed this in a392d80 Oct 22, 2021
@philwebb
Copy link
Member

Thanks very much @vignesh1992, this has now been merged to main. I've made a slight tweak to allow camelCase names to be used but only kebab-case to be advertised in the help.

humaolin pushed a commit to humaolin/spring-boot that referenced this pull request May 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants