Skip to content

Conversation

@HugoMario
Copy link
Contributor

No description provided.

@HugoMario HugoMario merged commit 78e6442 into master Mar 30, 2018
@jmini
Copy link
Contributor

jmini commented Mar 30, 2018

Reverting work made in #43 without discussing it feels wrong. You let some PR opened for days and now you are reverting some work without asking.

Some java generators are using the values:

{{modelPackage}}
{{apiPackage}}
{{invokerPackage}}

I am not sure that the values will still be available in the templates.

@HugoMario
Copy link
Contributor Author

I understand your point, and i'll updates generators that use those values so we won't notice any difference.

Sorry about my rush, we can talk about it and i can revert it. I'm just trying to avoid issues for new generators migrated.

@HugoMario
Copy link
Contributor Author

for generators that initializes the {{modelPackage}} and {{apiPackage}} values (like php) from constructor

https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/PhpClientCodegen.java#L66-L67

The reverted commit was affecting the {{modelPackage}} value
https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/PhpClientCodegen.java#L245

and the {{apiPackage}} value https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/PhpClientCodegen.java#L251

That's the main reason to revert commit, now we just need to add those values for additionalProperties in the specific generator instead the default generator class (DefaultCodegenConfig)

@jmini
Copy link
Contributor

jmini commented Apr 3, 2018

I have described the requested feature here: #55.


For consistency reasons I am still convinced that the way I implemented it is way better. Because of one special case, you are implying that every generator should do the “if map doesn’t contain key then use the member value“ check. This feels wrong because modelPackage is defined at DefaultCodegenConfig level. The default handling, that will be good for 90% of the cases.


As for your custom case in the PhpClientCodegen class, I think that the solution is to do:

@Override
public void processOpts() {

    // <1> custom handling, that needs to be made before the default handling.

    super.processOpts();

    // <2> other handling.
}

Move your custom logic for PHP in <1> instead of <2> and it will work.

I have used this pattern in the AbstractJavaCodegen class:
AbstractJavaCodegen, Lines 170..187 => This is before the super.processOpts(); call.


Please merge #56 to revert this and I will help you with the PHP generator.

@HugoMario HugoMario deleted the revert-commit-e23815 branch May 31, 2018 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants