-
Notifications
You must be signed in to change notification settings - Fork 6k
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
[C#][Nancy] Customize interface prefix #4557
Conversation
Per swagger-api#4486, this allows user to specify the use of a standard or custom prefix for interfaces. For C# based languages, this follows Microsoft's Framework Design Guidelines and uses an I- prefix. However, to avoid breaking changes with existing nancyfx generated code, the default is unset. The option supports true, false, or a custom prefix.
@@ -254,6 +256,18 @@ public void processOpts() { | |||
if (additionalProperties.containsKey(CodegenConstants.OPTIONAL_EMIT_DEFAULT_VALUES)) { | |||
setOptionalEmitDefaultValue(Boolean.valueOf(additionalProperties.get(CodegenConstants.OPTIONAL_EMIT_DEFAULT_VALUES).toString())); | |||
} | |||
|
|||
if (additionalProperties.containsKey(CodegenConstants.INTERFACE_PREFIX)) { | |||
String useInterfacePrefix = additionalProperties.get(CodegenConstants.INTERFACE_PREFIX).toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggestion: what about using Boolean.valueOf
to store CodegenConstants.INTERFACE_PREFIX
as boolean instead of string so as to avoid case sensitivity issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wing328 Boolean.valueOf
would return true if true, and false otherwise. This would remove support for custom prefixes, as in the example in the PR description which prefixes with ZZ
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jimschubert you're right. I missed that.
Per swagger-api#4486, this allows user to specify the use of a standard or custom prefix for interfaces. For C# based languages, this follows Microsoft's Framework Design Guidelines and uses an I- prefix. However, to avoid breaking changes with existing nancyfx generated code, the default is unset. The option supports true, false, or a custom prefix.
PR checklist
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
and./bin/security/{LANG}-petstore.sh
if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)2.3.0
branch for breaking (non-backward compatible) changes.Description of the PR
Per #4486, this allows user to specify the use of a standard or custom
prefix for interfaces. For C# based languages, this follows Microsoft's
Framework Design Guidelines and uses an I- prefix. However, to avoid
breaking changes with existing nancyfx generated code, the default is
unset.
The option supports true, false, or a custom prefix.
To test:
NancyFX:
NOTE: true/false don't apply here because there is no default set. This assumes NancyFX community preference is no prefix.
C# Client
Questions...
I'd rather have NancyFX default to the I prefix and allow for the generator to also support the true/false/custom in the same way as the C# generator here. If this is cool, we'll just need to remove the
setInterfacePrefix("")
line (and its comment) inNancyFXServerCodegen
.