Add a check enforce "samples/" folder to be up-to-date

[jmini]

I think that with each PR, the samples/ needs to be completely updated. For the moment people focus only on their generators (which is fine), but this is wrong.

First reason: some CI Jobs are updating /samples with /bin/run-all-petstore and then run each generated project. So they are not executed against code that is checked in, but against code that is generated.

Second reason: for later analysis (understand what was broken when) it is good to have the changes in the same PR.

Third reason: even if you think that you have only updated one generator (say java) your change might have an undesired impact in a other generator. Having it explicitly is good. This is even worth when you change something in DefaultGenerator (see my mistake in #79)

---

Pre-requisit:

• All generators needs to support the hide generation timestamp feature.
• All examples in the samples/ project needs to have been generated once

---

The check should looks like this and could be added at the end of bin/run-all-petstore:

if ! `git diff-index --quiet HEAD -- `; then
    echo "There are uncommitted changes in working tree after execution of 'bin/run-all-petstore'"
    git status
    echo "Please commit changes"
    exit 1
else
    echo "Git working tree is clean"
fi

[jmini]

This seems to be really complicated to do right now because we still have work to have all generator generated by our generator.

I instead of using bin/run-all-petstore we could have an other a sh script (bin/run-important-petstore) that calls our most important bin/*.sh scripts. The list could grow over time and at the end it could be similar to bin/run-all-petstore.

After run of bin/run-important-petstore, there should be no modification the git working tree, because all modifications should be included in the PR.

Maybe this will only run locally at the beginning (and not enforce at CI level), but this will be a good start giving more confidence when we work on stuff in DefaultCodegen.

[jonschoning]

currently all the scripts in bin/openapi3 also target samples/client/petstore, so effectively they overwrite each other depending on which you choose.

[tomplus]

Definitely it's necessary and adding the check to CI is good enough for me.

I agree that some generators are not ready yet. We skip these in unit tests and so we can also skip it here too. I'm thinking about defining some kind of 'maturity level' for generators. If a technical committee decides that their generator is production-ready, they will add this to all tests cases.

[jmini]

Ok what should be the list of mature generator that we want to use as input to force the update of samples/ folder?

[jmini]

I have made great progress on a first version of a bin/ensure-up-to-date script.
"Shippable CI" now as first step ensure that samples/ is up to date for a list of generators.
The list of bin/*.sh scripts is defined in the bin/ensure-up-to-date itself.
=> See PR #136

---

In my opinion the list scripts executed during this check can evolve over time. For the beginning, we should select:

• Mature generators.
• Generator supported by people doing PR review regularly (because if something evolves in a generated sample and feedback is requested on the PR to tell if it is good or bad).

I wanted to add more items to the list:

• generators used by a lot of users
• programming languages that are well known

But I have the feeling that those items are not measurable and will create too much debate...

---

Feel free to give your opinion.

[jmini]

As mentioned in #136, we can keep this issue open and propose new PRs to add other generators in the list.

[ackintosh]

Totally agree with adding the check. I'll add generator that I'm reviewing regularly to the list.

As mentioned in #80 (comment) , the list will grow over time and the target generator could be similar to run-all-petstore .
When it comes, we need to consider running run-all-petstore instead of {LANG}-petstore.sh on the PR as only differences caused by the PR will be generated by run-all-petstore.

[jmini]

@macjohnny, @TiFu: I have noticed a lot of activity/review around the typescript generator.

If you are interested by the feature described here (checking with each PR that the generator runs and that its result included in a commit as part of the same PR to make the Shippable build green), feel free to add the necessary bin/*.sh scripts in the list:

openapi-generator/bin/utils/ensure-up-to-date

Lines 12 to 27 in dcc0c17

	# LIST OF SCRIPTS:
	./bin/ruby-petstore.sh > /dev/null 2>&1
	./bin/java-petstore-all.sh > /dev/null 2>&1
	./bin/java-jaxrs-petstore-server-all.sh > /dev/null 2>&1
	./bin/spring-all-pestore.sh > /dev/null 2>&1
	./bin/kotlin-client-petstore.sh > /dev/null 2>&1
	./bin/kotlin-client-string.sh > /dev/null 2>&1
	./bin/kotlin-client-threetenbp.sh > /dev/null 2>&1
	./bin/kotlin-server-petstore.shl> /dev/null 2>&1
	./bin/php-petstore.sh > /dev/null 2>&1
	./bin/php-silex-petstore-server.shj> /dev/null 2>&1
	./bin/php-symfony-petstore.sh > /dev/null 2>&1
	./bin/php-lumen-petstore-server.sh > /dev/null 2>&1
	./bin/php-slim-petstore-server.sh > /dev/null 2>&1
	./bin/php-ze-ph-petstore-server.sh > /dev/null 2>&1
	./bin/openapi3/php-petstore.sh > /dev/null 2>&1

[jimschubert]

#4030 is an in-progress work to clean up the samples directory.
#3789 is an initial take on generating the samples in batch.

I think there are a few issues currently with generating the samples directory:

• It take a very long time to run the ensure-up-to-date script (attempting to address this with my batch PR)
• We only overwrite previous files, we don't clean up old files… this means that a change in how we name files (either via code manipulation, or changes to yaml spec doc) may result in compilation errors that OSS contributors may not understand or care to spend time resolving on their PRs.
• People seem to find the output directory confusing; is it samples? is it regression tests? is it compilation tests?
• It's not all-inclusive. How do we know when a "sample" is ready to be considered a sample for the ensure-up-to-date script?

Speaking with @Fjolnir-Dvorak on Slack, I suggested that we may want to consider outputting the list of generated files to a text file under the .openapi-generator directory (as we do currently with VERSION). We could automate recreation of samples using this output, and rather than having the samples.ci directory which caches non-generated sources, we could make use of .openapi-generator-ignore and persist these directly in the samples directory. To do this, we'd need:

• An opt-in option to write out the files created by the generator (

  openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultGenerator.java

  Line 919 in bf9f47c

  List<File> files = new ArrayList<File>();

  )
• A modification to ensure-up-to-date to delete files listed in the above file
• Change ensure-up-to-date to use the batch generation option
• Move all sample.ci files to their respective output directories, and adjust any relevant scripts
• Update documentation

We could then use Github Actions to run ensure-up-to-date (I believe).