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

Implement GitHub Actions workflow for automatically updating the example JSON files #2068

Closed
wants to merge 4 commits into from

Conversation

cebe
Copy link
Contributor

@cebe cebe commented Nov 27, 2019

As discussed in #1385 the example .json files for version 3 should be automatically generated from the .yaml example files.

I thought this was a good case for trying out Github Actions and the result is the following:

  • When a push to master (or any other branch we wish to include) is made, a Github Actions Workflow is triggered (example)
  • it will generate new JSON files from the YAML examples
  • if there are any changes, it will create a pull request to the branch that triggered the Workflow (example)

Copy link
Member

@webron webron left a comment

Choose a reason for hiding this comment

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

Huh, this is awesome. Would it make sense to change the Action to run on *.yaml in that directory instead?

push:
branches:
- master
- try-github-actions
Copy link
Member

Choose a reason for hiding this comment

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

Is that still needed? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably not, going to remove that before merging this PR. But it will run the action if I make adjustments to this branch so I'd keep it for testing until it is ready to merge.

@cebe
Copy link
Contributor Author

cebe commented Nov 27, 2019

Would it make sense to change the Action to run on *.yaml in that directory instead?

what do you mean? As far as I am aware there is no way to run an action based on whether some files changed or not.

@cebe
Copy link
Contributor Author

cebe commented Dec 4, 2019

@webron is there anything to change or to do from my side to get this merged? (except from removing the try-github-actions branch)

@webron
Copy link
Member

webron commented Dec 4, 2019

Sorry, I didn't mean run it based on whether a file changed, but rather here:

        vendor/bin/php-openapi convert --read-yaml callback-example.yaml  --write-json callback-example.json
        vendor/bin/php-openapi convert --read-yaml link-example.yaml      --write-json link-example.json
        vendor/bin/php-openapi convert --read-yaml petstore.yaml          --write-json petstore.json
        vendor/bin/php-openapi convert --read-yaml petstore-expanded.yaml --write-json petstore-expanded.json
        vendor/bin/php-openapi convert --read-yaml uspto.yaml             --write-json uspto.json

instead of running it on each file individually, it'd be more robust if it ran on all yaml file in the directory, in case additional files are added.

@MikeRalphson
Copy link
Member

Also, what indentation is used in the output json? I guess it should match the v2.0 examples for consistency.

@cebe
Copy link
Contributor Author

cebe commented Dec 5, 2019

Indentation is currently 4 spaces, I'll change that to 2.

@MikeRalphson
Copy link
Member

@cebe is the exclusion due to #2036 ?

Just a thought for after this is merged, would you have time to create a similar workflow to keep the json versions of the yaml json-schemas in step? It would need to run on versions (directories) 3.0 and above.

@cebe
Copy link
Contributor Author

cebe commented Dec 5, 2019

@cebe is the exclusion due to #2036 ?

yes, symfony/yaml has some problems with these files, did not see that PR, will try to merge the commit and see if it fixes the problem.

Just a thought for after this is merged, would you have time to create a similar workflow to keep the json versions of the yaml json-schemas in step? It would need to run on versions (directories) 3.0 and above.

sure I can do that.

This workflow updates the *.json files in the examples/v3.0 directory,
when the corresponding *.yaml files change.
JSON example files are automatically generated from the YAML example
files.
Only the YAML files should be adjusted manually.

fixes OAI#1385

- When a push to `master` (or any other branch we wish to include) is
made, a Github Actions Workflow is triggered
([example](https://github.com/cebe/OpenAPI-Specification/commit/9c98e819ae876af92c2a9112dcfa6dfcb929e7dc/checks?check_suite_id=331067708))
- it will generate new JSON files from the YAML examples
- if there are any changes, it will create a pull request to the branch
that triggered the Workflow
([example](#3))
@cebe cebe force-pushed the try-github-actions branch from 1572979 to 22c0653 Compare December 6, 2019 08:28
@cebe cebe force-pushed the try-github-actions branch from 4db07c7 to 2d1e210 Compare December 6, 2019 13:42
@cebe
Copy link
Contributor Author

cebe commented Dec 6, 2019

Merged the changes from #2036, they fix the issue with api-with-examples.yaml.

instead of running it on each file individually, it'd be more robust if it ran on all yaml file in the directory, in case additional files are added.

done.

Also, what indentation is used in the output json? I guess it should match the v2.0 examples for consistency.

indentation is 2 now.

Ready to merge from my side.

@cebe cebe force-pushed the try-github-actions branch from 2d1e210 to d1ed602 Compare December 6, 2019 13:46
@cebe cebe requested a review from webron December 6, 2019 13:47
Copy link
Member

@webron webron left a comment

Choose a reason for hiding this comment

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

I'm ok with the changes. @MikeRalphson please verify as well. If It's all good, we need to remove the test branch from the script and then merge it.

@MikeRalphson
Copy link
Member

My only question is why we indent the target with tabs and then (presumably) overwrite it with a space indented version. Otherwise LGTM.

@cebe
Copy link
Contributor Author

cebe commented Dec 9, 2019

My only question is why we indent the target with tabs and then (presumably) overwrite it with a space indented version.

The tool I use for indentation can not convert 4 space to 2 space indentation. it can only convert tabs to spaces or spaces to tabs. So I do the conversion in two steps, 4 spaces to tabs, then tabs to 2 spaces.

@cebe
Copy link
Contributor Author

cebe commented Dec 9, 2019

we need to remove the test branch from the script

done.

@MikeRalphson MikeRalphson self-requested a review December 9, 2019 09:31
@MikeRalphson
Copy link
Member

@webron are you ok to create the GITHUB_TOKEN before merging?

@webron
Copy link
Member

webron commented Dec 10, 2019

@MikeRalphson If I understand correctly, GITHUB_TOKEN is created automatically when GitHub Actions are enabled (which they are). See https://help.github.com/en/actions/automating-your-workflow-with-github-actions/authenticating-with-the-github_token.

@MikeRalphson
Copy link
Member

👍

@cebe
Copy link
Contributor Author

cebe commented Dec 23, 2019

there is no need to add a github token manually, it is provided by Github actions. Anything stopping this from being merged?

@cebe
Copy link
Contributor Author

cebe commented May 27, 2020

Closing this, as #2199 is replacing it.

@cebe cebe closed this May 27, 2020
@cebe cebe deleted the try-github-actions branch April 20, 2021 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants