async-stripe icon indicating copy to clipboard operation
async-stripe copied to clipboard

Add command line interface for controlling OpenAPI spec version used in codegen

Open mzeitlin11 opened this issue 2 years ago • 1 comments

This PR adds a small command line interface for the codegen process. The main motivation for the change was to allow controlling the OpenAPI specification version used, so that cargo make openapi-install can always produce the same result as on the current master branch, which could not be guaranteed previously since before the fetch was against the latest commit in the stripe repo.

Specifically, this pr adds the optional argument --fetch with the possible values:

  • current: Use the OpenAPI spec version used to generate the current generated code.
  • latest: Use the latest OpenAPI release
  • v{}: Target a specific OpenAPI release.

This could also enable adding future consistency checks to the CI, such as a check that the generated code is up to date with a PR, by comparing generated PR code to cargo make openapi-install since spec versions can now be guaranteed to match.

mzeitlin11 avatar Jul 30 '22 22:07 mzeitlin11

Codecov Report

Merging #255 (d2290d1) into master (c1ed246) will not change coverage. The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #255   +/-   ##
======================================
  Coverage    5.41%   5.41%           
======================================
  Files         148     148           
  Lines       14231   14231           
======================================
  Hits          770     770           
  Misses      13461   13461           

Help us with your feedback. Take ten seconds to tell us how you rate us.

codecov[bot] avatar Jul 30 '22 23:07 codecov[bot]

Hi, sorry for the delay here.

arlyon avatar Aug 15 '22 14:08 arlyon

This is an excellent change, I will review shortly.

arlyon avatar Aug 15 '22 14:08 arlyon

This is great, I had a question around the version file, I take it the goal there is to declaratively specify the version we are using in the repo rather than always fetching latest? I think making it clear in the repo what version we are generating against is the right move but I want to make sure we consider how that impacts the openapi generator action (https://github.com/arlyon/async-stripe/blob/master/.github/workflows/openapi.yml).

I am happy to merge as soon as we get a plan in place (that may just be having it create a commit to bump the version before the rest of the workflow)

arlyon avatar Aug 16 '22 09:08 arlyon

I think making it clear in the repo what version we are generating against is the right move but I want to make sure we consider how that impacts the openapi generator action (https://github.com/arlyon/async-stripe/blob/master/.github/workflows/openapi.yml).

The goal with this change was to maintain behavior for that action (though caveat not sure of a good way to test this :)) There should now be 2 cargo make targets with the following behavior:

  • openapi-install: fetch + generate code for the version in versions.json
  • openapi-install-latest: fetch the latest OpenAPI version and generate code for that. This should also update versions.json to that version.

So with these changes the OpenAPI generator action now uses the target openapi-install-latest to maintain behavior of trying to generate code for the latest OpenAPI version.

mzeitlin11 avatar Aug 16 '22 14:08 mzeitlin11

Cool, lets come up with a strategy for updating that version as well.

arlyon avatar Aug 16 '22 16:08 arlyon

Merging, ty !:tada:

arlyon avatar Aug 16 '22 16:08 arlyon