Skip to content

Conversation

@bartkummel
Copy link
Contributor

@bartkummel bartkummel commented Feb 14, 2017

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.

See issue #4796

Description of the PR

Improved ExampleGenerator:

  • Now takes into account enum and uri/url formats for strings.
  • Uses example for referenced objects if available.
  • Proper examples get generated for specific numeric formats, because more specific formats now get checked before generic format.
  • Honors min and max values for numerical properties, if set.

Bart Kummel added 2 commits February 14, 2017 15:10
- Now takes into account enum and uri/url formats for strings.
- Uses example for referenced objects if available.
- Proper examples get generated for specific numeric formats, because more specific formats now get checked before generic format.
- Honors min and max values for numerical properties, if set.
import java.util.Set;

public class ExampleGenerator {
private static final Logger log = LoggerFactory.getLogger(ExampleGenerator.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bartkummel thanks for the PR. Just a minor suggestion. We use logger instead of log in other Java class files. Do you mind using the same (ie. logger) to make it consistent with other files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! I've renamed log to logger in the latest commit.

@wing328
Copy link
Contributor

wing328 commented Mar 3, 2017

@bartkummel thanks for the PR. I like the new feature you added: randomized example values (numeric), but this may introduce extra noise during change review. I'll merge this and see if the community also has feedback on this.

@wing328 wing328 merged commit 30c2b6f into swagger-api:master Mar 3, 2017
@ePaul
Copy link
Contributor

ePaul commented Mar 14, 2017

The random integers have the effect that now after each generation we get different numbers. Currently those examples seem to be not used in many languages (just NodeJS, as far as I can see), but it is already annoying (e.g. see ccc117c for how a simple nothing-changing update looks).

If we need randomness, could we make it deterministic? I.e. start the RNG always with the same seed (per ExampleGenerator instance, e.g.), so same input gets same output? This would turn down the noise.

ePaul pushed a commit to ePaul/swagger-codegen that referenced this pull request Mar 14, 2017
spr3nk3ls pushed a commit to spr3nk3ls/swagger-codegen that referenced this pull request Mar 28, 2017
* Improved ExampleGenerator:

- Now takes into account enum and uri/url formats for strings.
- Uses example for referenced objects if available.
- Proper examples get generated for specific numeric formats, because more specific formats now get checked before generic format.
- Honors min and max values for numerical properties, if set.

* Ran script `bin/nodejs-petstore-server.sh`.

* Renamed log to logger to conform to coding standard.
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.

3 participants