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

Spring-CLI: "packageName" parameter is not aligned with the docs and doesn't fit the naming convention of other parameters #26878

Closed
skryvets opened this issue Jun 12, 2021 · 13 comments
Labels
status: first-timers-only An issue that can only be worked on by brand new contributors status: superseded An issue that has been superseded by another type: bug A general bug

Comments

@skryvets
Copy link

skryvets commented Jun 12, 2021

Steps to reproduce:

  1. In the command line, run spring init --list. As a result a list of dependencies and parameters will appear in the console:
..... 
(omitted for brevity)
....

Parameters
+-------------+------------------------------------------+------------------------------+
| Id          | Description                              | Default value                |
+-------------+------------------------------------------+------------------------------+
| artifactId  | project coordinates (infer archive name) | demo                         |
| bootVersion | spring boot version                      | 2.5.1                        |
| description | project description                      | Demo project for Spring Boot |
| groupId     | project coordinates                      | com.example                  |
| javaVersion | language level                           | 11                           |
| language    | programming language                     | java                         |
| name        | project name (infer application name)    | demo                         |
| packageName | root package                             | com.example.demo             |
| packaging   | project packaging                        | jar                          |
| type        | project type                             | maven-project                |
| version     | project version                          | 0.0.1-SNAPSHOT               |
+-------------+------------------------------------------+------------------------------+
  1. Using the suggested docs, try to run init a spring app with a custom package name, e.g. :
spring init --packageName=com.skryvets.demo spring-demo

Expected result:

Spring app should be created. The following (or similar output) should be produced in the console:

Using service at https://start.spring.io
Project extracted to '/yourfolder/spring-demo'

Actual result:

Spring app creation fails with the following message: packageName is not a recognized option

Notes:

The reason I put "doesn't fit the naming convention of other parameters" in the description as it seems like all other options are "camel cased" whereas this particular one is dash separated. There are 2 way of fixing - either changing the docs or parameter name. I felt like changing the parameter is more appropriate in this case.

@skryvets
Copy link
Author

This is my first issue as well as PR that I submitted here: #26879. Please correct me if I wrong.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 12, 2021
@snicoll
Copy link
Member

snicoll commented Jun 13, 2021

The reason I put "doesn't fit the naming convention of other parameters" in the description as it seems like all other options are "camel cased" whereas this particular one is dash separated.

java-version, boot-version are also camel case to follow the convention of the CLI, although groupId and artifactIddon't so that's a bit odd.

If you run spring help init you'll see how you can build a command (either with a shorthand, i.e. -p for the package name or the full name). The thing that is a bit confusing here is that the server gives you back a number of "parameters" that it supports primarily to tell you about default values.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Jun 13, 2021
@skryvets
Copy link
Author

java-version, boot-version are also camel case to follow the convention of the CLI, although groupId and artifactId don't so that's a bit odd.

You're 100% correct. Even remembering a couple of parameters might help, but knowing which one should be camelCased and which one should not, makes it harder to use (unless you would look in the documentation).

Talking about documentation and the --package-name specific parameter. I see you mentioned about spring help init and after running it I see it outputs --package-name. However, I also double checked what comes from the server by triggering (curl --request GET --url https://start.spring.io/ --header 'Accept: application/json') and I found nested camelCased packageName json object.

It seems like the best option would be to change the camelCase naming on the server to the dash-separated. However, it's clear that it might break some existing clients/integrations.
An alternative approach is to map all camelCasing that comes from the server to a dash-separated parameters in the CLI.

I would wait for your feedback. Thanks a lot for looking at this.

@snicoll
Copy link
Member

snicoll commented Jun 14, 2021

However, I also double checked what comes from the server by triggering (curl --request GET --url https://start.spring.io/ --header 'Accept: application/json') and I found nested camelCased packageName json object.

That's a totally different endpoint that provides the API contract.

As I've mentioned in my reply, the server gives you back a number of "parameters" that it supports primarily to tell you about default values. These are not meant to map to the actual option that you can use using the Sping CLI. Arguably, if it was we should rather have written them as --package-name. An improvement would be the change that Id column to a name to make it more obvious. Alternatively, we could indeed change the content on the server to present the actual option. I've flagged for team attention to get more feedback from the team.

@wilkinsona
Copy link
Member

wilkinsona commented Jun 14, 2021

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. It might also be worth filtering this information so that an option and its default value is only showing if the CLI supports it.

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.

@philwebb philwebb added status: reserved-for-conference-event type: bug A general bug and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Jul 12, 2021
@philwebb philwebb added this to the 2.4.x milestone Jul 12, 2021
@scottfrederick scottfrederick added status: first-timers-only An issue that can only be worked on by brand new contributors and removed status: reserved-for-conference-event labels Sep 1, 2021
@vignesh1992
Copy link
Contributor

Hi Team, wanted to check if this issue is still open to contribute?
Is it expected to allow both CLI options (kebab-case) and query parameters (camelCase) for the current milestone or just to support only CLI options (kebab-case)?
Looking forward towards your feedback.

@snicoll
Copy link
Member

snicoll commented Sep 27, 2021

Yes, it is. The comment above your summarizes what we'd like to do. We can't stop supporting an option without a deprecation period so, yes, both CLI options should work. You can submit a PR if you like and we can guide you from there.

@vignesh1992
Copy link
Contributor

Hi @snicoll, Thank you so much. I have opened a PR 28138. Since it is my first issue fix, request you to kindly guide me if there are anything more which I could contribute. Looking forward towards your feedback!

@wilkinsona
Copy link
Member

Closing in favour of #28138.

@wilkinsona wilkinsona added the status: superseded An issue that has been superseded by another label Sep 30, 2021
@wilkinsona wilkinsona removed this from the 2.4.x milestone Sep 30, 2021
@vignesh1992
Copy link
Contributor

Hi @wilkinsona, Request you to kindly provide the issue/PR which supersedes the fix for #28138? Thank you so much

@philwebb
Copy link
Member

@vignesh1992 #28138 has not been superseded. We're closing this issue in favor of your PR.

@vignesh1992
Copy link
Contributor

Hi @philwebb, Thank you so much for your response. As this is my first time contributing to this repo, would be interested to know to get feedbacks on the PR and next steps on when it will make it to the release.

@philwebb
Copy link
Member

@vignesh1992 We hope to get to it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: first-timers-only An issue that can only be worked on by brand new contributors status: superseded An issue that has been superseded by another type: bug A general bug
Projects
None yet
Development

No branches or pull requests

7 participants