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

typescript-angular: Make the api suffix configurable #234

Closed
macjohnny opened this issue Jun 6, 2018 · 13 comments
Closed

typescript-angular: Make the api suffix configurable #234

macjohnny opened this issue Jun 6, 2018 · 13 comments

Comments

@macjohnny
Copy link
Member

See original issue at swagger-api/swagger-codegen#8281

@macjohnny
Copy link
Member Author

@tomvangreen would you like to implement this here at OpenAPI Generator?

@tomvangreen
Copy link
Contributor

I have to implement it for swagger codegen, as that is what we use in our project(s) and I cannot change it at this time. But I can see how hard it is to also provide it for openapi-generator. Can't guarantuee anything, but I will see.

@macjohnny
Copy link
Member Author

@tomvangreen by the way, migration to openapi-generator is as simple as replacing the dependency in your pom.xml:

<dependency>
    <groupId>org.openapitools</groupId>
    <artifactId>openapi-generator-maven-plugin</artifactId>
    <version>${openapi-generator-version}</version>
</dependency>

I did that recently in our project, and it worked like a charm.

I would be happy to review your PR with the changes.

@tomvangreen
Copy link
Contributor

I have added two parameters for the typescript angular code generator, serviceSuffix and serviceFileSuffix. I tried running the tests and I get an error, that those two parameters have not been tested. I looked through the repo to try to find out where or how I can test those new parameters, but I am a bit lost.

Can anybody point me in the right direction for this?

@macjohnny
Copy link
Member Author

@tomvangreen what is the error message?
@wing328 any idea where to fix this?

@tomvangreen
Copy link
Contributor

@macjohnny Sorry, I could have added the message right away. I get this error message:
Failed tests: TypeScriptAngularClientOptionsTest>AbstractOptionsTest.checkOptionsHelp:61 These options weren't checked: serviceSuffix, serviceFileSuffix.

@tomvangreen
Copy link
Contributor

I think I got it. I needed to add the parameters in the TypeScriptAngularClientOptionsProvider.

@tomvangreen
Copy link
Contributor

@macjohnny I think this issue can be closed, as my pull request has been merged.

But before you close it, maybe I could hijack this issue for another related topic/questsion. One other point of pain for us is the naming of the model classes. The swagger api we are using is also used by other clients and we don't have much control over how the models are tagged/named.

I was thinking that maybe I could add a configuration option to make the model suffix also configurable (for example, instead of having a file user.ts with the class User, we could configure the suffix Vm and file suffix .vm in order to get the file user.vm.ts with the class UserVm). Do you think a config param like that would make sense? I think in most cases the better solution would probably be to simply give the right name in the swagger schema.

If there is no objection I could have a look if I can implement that configuration.

@macjohnny
Copy link
Member Author

@tomvangreen if there is a need to control the generated model names, of course, we should have an option for that. If you would want to contribute this feature, this would be great.

@macjohnny
Copy link
Member Author

maybe @TiFu has an opionion on that? would this also be useful for the other typescript generatorS?

@TiFu
Copy link
Contributor

TiFu commented Jun 29, 2018

Sorry, I have been insanely busy the past couple weeks.

I don't see any problem with adding this option as long as it defaults to not adding a suffix. It might also be useful for other generators even if the model classes are not put into different files.

@tomvangreen
Copy link
Contributor

tomvangreen commented Jun 29, 2018

I'm not sure how simple it is to implement this also for the other generators. I only had a look into the typescript angular one, but if it's not too much work I see if I can do the change for the other generators as well.

If I can implement the change I will make sure that you will have the same output as before if you don't set the option.

@macjohnny
Copy link
Member Author

implemented with #418

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants