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

[C#][Nancy] Customize interface prefix #4557

Merged
merged 1 commit into from
Jan 15, 2017

Conversation

jimschubert
Copy link
Contributor

PR checklist

  • Read the contribution guildelines.
  • Ran the shell/batch script under ./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)
  • Filed the PR against the correct branch: master for non-breaking changes and 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:

./bin/nancyfx-petstore-server.sh --additional-properties interfacePrefix=I

NOTE: true/false don't apply here because there is no default set. This assumes NancyFX community preference is no prefix.

C# Client

# Default behavior, prefix with I-
./bin/csharp-petstore.sh --additional-properties interfacePrefix=true

# Customize without the default prefix
./bin/csharp-petstore.sh --additional-properties interfacePrefix=false

# Customize using your own prefix
./bin/csharp-petstore.sh --additional-properties interfacePrefix=ZZ

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) in NancyFXServerCodegen.

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.
@wing328 wing328 added this to the v2.2.2 milestone Jan 15, 2017
@wing328 wing328 merged commit 663b471 into swagger-api:master Jan 15, 2017
@@ -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();
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

@jimschubert jimschubert deleted the nancyfx/4486 branch January 15, 2017 16:42
@wing328 wing328 changed the title [nancyfx/csharp] Customize interface prefix [C#][Nancy] Customize interface prefix Feb 20, 2017
davidgri pushed a commit to davidgri/swagger-codegen that referenced this pull request May 11, 2017
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.
@craffael craffael mentioned this pull request Sep 29, 2017
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants