-
Notifications
You must be signed in to change notification settings - Fork 3
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
Ship Open API CLI as NPM package with generator #194
Comments
I don't like that a user can't use this NPM package without having Java installed and added to path. For that reason I would vote for checking the OpenAPI generator version and showing an error, if the installed major version is not the expected one. (If generator expect version 6.X, but you have 7.X installed, you get a proper warning for this) When we check the version, we can let it up to the user how to install the OpenAPI generator - for example I like the Homebrew way, which is just working. When we check for the version and the OpenAPI generator is not installed, we ask the user to install it and give the link https://openapi-generator.tech/docs/installation to him. What do you think? |
@robinmanuelthiel added the openapi-generator check: generator/src/sdk-auto-generate-dotnet/templates/script.sh Lines 13 to 25 in 015118a
|
As far as I can tell, this script checks the Major AND Minor version whereas you only wanted to pin the Major Version, right? |
Was not sure, if a change in minor could also be a breaking change - but if this is not the case we could replace it with sth. like |
Yes the CLI is Java and the user needs to have Java installed. But why is it a difference from installing it via Homebrew? Does Homebrew install Java automatically, whenever I install the OpenAPI CLI? If it doesn't, I would say it's same same and I like the Idea that installing our Generator via NPM is all we need to ensure it is running as expected, because it brings the EXACT version of the CLI that we tested For this reason, that we exactly know the version of the CLI that is used, I would still vote for adding it as a dependency as discussed in this issue from the beginning |
I don't know where the requirement comes from... |
The requirement is the whole reason we have this issue. I had a different version of the CLI installed than you. This cannot happen anymore, when we ship the CLI with the generator.
This doens't make sense to me. Telling the user that something is missing is EXACTLY what we do with the script you wrote... |
(Just a sum-up for the next person who picks it up) TLDR: Remove Open API CLI version check from Script and ship it as an NPM dependency and make sure that the version of the Open API CLI gets used that comes from the NPM dependency, even if a different version of the NPM Open API CLI is installed locally. // If the user has installed the Open API CLI not via NPM (e.g. via Homebrew) we don't care, because only the NPM version has the name |
Different locally installed opencli-generator can lead to errors (see https://github.com/wemogy/authorization/issues/534)
Options
The text was updated successfully, but these errors were encountered: