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

tools/importer-rest-api-specs: splitting the dataapigeneratorjson package into transforms and stages #3831

Merged

Conversation

tombuildsstuff
Copy link
Contributor

@tombuildsstuff tombuildsstuff commented Feb 21, 2024

This PR refactors the dataapigeneratorjson package in two ways:

  1. Firstly it splits out the transformation logic (for mapping the resourcemanager types into the dataapimodels types) into it's own package (transforms).
  2. Secondly it updates the logic used to write the API Definitions to disk to use stages, using an intermediate filesystem type (similar to how the importer-msgraph-metadata logic works) - the intention here is to allow both testability (since we can validate the files exist at the paths we're expecting more easily) and that this type can be reused when loading the data from disk too.

This PR also makes the indentation consistent across all of the generated API Definitions - which means that when this PR is merged the regeneration is going to be fairly large but will only contain whitespace - but means that the API Definitions are consistent going forwards.

This PR represents a preparation stage for Phase 9 and 10 of #3754

…epository models out into it's own package

This both makes the migration to the new models simpler (since it's contained within one package) but also means the parent package can be tidied per Phase 10
…o split the transforms and file writing

This both enables reuse (for graph in time) and allows us to switch over to the Data API SDK models
…n` package to split the Generator and Transform stages
@tombuildsstuff tombuildsstuff force-pushed the refactor/9-arm-importer-transforms-and-sdk-models branch from eaef0a7 to 45db742 Compare February 21, 2024 16:45
Copy link
Member

@catriona-m catriona-m left a comment

Choose a reason for hiding this comment

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

LGTM!

}

// NOTE: `Parent` types don't get a `TypeHintIn` or `TypeHintValue`
// meaning that only
Copy link
Member

Choose a reason for hiding this comment

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

is this comment missing a bit 🤔 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is (it's a lift and shift) - but this is actually fixed in one of the follow up PR's so I'm gonna leave this for now

@tombuildsstuff tombuildsstuff merged commit 9a507b6 into main Feb 21, 2024
2 checks passed
@tombuildsstuff tombuildsstuff deleted the refactor/9-arm-importer-transforms-and-sdk-models branch February 21, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor tool/importer-rest-api-specs Swagger Data Importer issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants