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

[tsp-client] add combine-swaggers command #9017

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

chrisradek
Copy link
Member

This adds a quick and dirty combine-swaggers command to 'merge' multiple swagger files into a single swagger file. This is meant to make it easier to compare existing APIs split across multiple files with a TypeSpec-generated API that exists in a single file.

Only top-level fields are merged at this time, and no deduplication of array values is done.

Copy link
Member

@allenjzhang allenjzhang left a comment

Choose a reason for hiding this comment

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

Can we support filename wildcard?

Copy link
Member

@catalinaperalta catalinaperalta left a comment

Choose a reason for hiding this comment

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

Can we add some tests for this command to make sure nobody breaks current functionality in future updates?


const documents = [];
for (const file of swaggerFiles) {
if (!(await doesFileExist(file))) {
Copy link
Member

Choose a reason for hiding this comment

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

We should specify somewhere that this command only accepts the path to the swagger file and that we do not accept urls to a swagger since we do accept both in some commands.

continue;
}

// when b value is null and any a value exists, ignore b value
Copy link
Member

Choose a reason for hiding this comment

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

Isnt this a regression if we add the value of a back into the combined swaggers?

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.

3 participants