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

Move "ensure-up-to-date" execution to CircleCI #944

Merged
merged 3 commits into from
Aug 31, 2018

Conversation

jmini
Copy link
Member

@jmini jmini commented Aug 31, 2018

PR checklist

  • Read the contribution guidelines.
  • Ran the shell 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). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 3.3.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Extracted from PR #929:
@wing328 proposed to move execution of ensure-up-to-date (see #80) to CircleCI.

This PR does the change and I will perform some checks to ensure that it works as expected.

@@ -8,6 +8,10 @@ if [ "$NODE_INDEX" = "1" ]; then
cp CI/pom.xml.circleci pom.xml
java -version
mvn --quiet verify -Psamples
elif [ "$NODE_INDEX" = "2" ]; then
echo "Running node $NODE_INDEX to test ensure-up-to-date"
mvn clean package -DskipTests -Dmaven.javadoc.skip=true
Copy link
Member

Choose a reason for hiding this comment

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

"mvn clean install" was run before running this script (https://github.com/OpenAPITools/openapi-generator/pull/944/files#diff-29944324a3cbf9f4bd0162dfe3975d88R69) so I don't think we need to do it again or I probably miss the reason behind.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is just a matter of having modules/openapi-generator-cli/target/openapi-generator-cli.jar. I have put here the command that is sufficient to produce it (with a clean to be sure that it gets updated).

Copy link
Member Author

Choose a reason for hiding this comment

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

On shippable, there was already a mvn clean install, so what I did is just moving it before ensure-up-to-date with the initial commit:

00f176a#diff-033d042b39454e8e066ab38fd89610a5

Copy link
Member Author

Choose a reason for hiding this comment

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

Or do you mean here line 69:

# Test
- run: mvn --quiet clean install
- run: ./CI/circle_parallel.sh

OK I agree we do not need to redo it. I will remove the line.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@jmini
Copy link
Member Author

jmini commented Aug 31, 2018

Commit df18063 added to see if ensure-up-to-date is working. CircleCI should be red.

* bin/java-petstore-rest-assured.sh to update "samples/"
@jmini
Copy link
Member Author

jmini commented Aug 31, 2018

Error as expected:

circleci

Important: you need to switch to the second tab #queue-placeholder/containers/2 in the URL to see the error.

@jmini jmini merged commit f95dfbe into OpenAPITools:master Aug 31, 2018
@wing328 wing328 added this to the 3.3.0 milestone Aug 31, 2018
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
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