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

fix: openapi spec schema.type #1023

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

youngjungithub
Copy link
Contributor

@youngjungithub youngjungithub commented Mar 14, 2025

Query parameters in the OpenAPI specifications are incorrectly defined as type: string, even when they represent other data types such as integers (e.g., count=100).

This misrepresentation causes confusion for API consumers, as the specs do not accurately reflect the expected data types. It also undermines proper validation and the quality of auto-generated documentation.

@youngjungithub youngjungithub requested a review from a team as a code owner March 14, 2025 16:00
@youngjungithub youngjungithub marked this pull request as draft March 14, 2025 16:00
@youngjungithub youngjungithub changed the title fix: openapi spec schmema.type fix: openapi spec schema.type Mar 14, 2025
@youngjungithub youngjungithub force-pushed the queryparam-rework branch 2 times, most recently from e37f6fa to 75b09a0 Compare March 17, 2025 18:43
@youngjungithub youngjungithub marked this pull request as ready for review March 18, 2025 14:32
@youngjungithub youngjungithub marked this pull request as draft March 18, 2025 14:35
@youngjungithub youngjungithub marked this pull request as ready for review March 18, 2025 14:37
@ericcrosson-bitgo ericcrosson-bitgo marked this pull request as draft March 20, 2025 16:33
@ericcrosson-bitgo
Copy link
Contributor

ericcrosson-bitgo commented Mar 20, 2025

@youngjungithub, I've put this back into draft until the following action items are complete:

  1. Please rebase on latest master -- I see merge conflicts locally stemming from prettier formatting
  2. Please combine all commits into one, as a way to improve the signal-to-noise ratio of the commit log
  3. Please improve the commit message, which you can then use as the PR body.
    Recall that our release pipeline uses your commit messages to derive release notes, so we should write commit messages to contain the information we want our release notes to contain. Here is an example of a good release note -- it includes a summary of the change, and the backing motivation; it helps the reader visualize which parts of an apiSpec will generate different OpenAPI output, and how that output has changed.

…parameter support

Corrected a typo in knownImports.ts and enhanced query parameter generation logic ensuring compliance with
OpenAPI 3.0.3 specifications.
These changes produce a more accurate and complete OpenAPI spec,
with query parameters now properly documented alongside schema types, improving API
usability and clarity for downstream consumers.
@youngjungithub youngjungithub marked this pull request as ready for review March 20, 2025 17:43
Copy link
Contributor

@ericcrosson-bitgo ericcrosson-bitgo left a comment

Choose a reason for hiding this comment

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

Outstanding work! You turned this into a readable addition, which in turn will make our OpenAPI output more readable and idiomatic.

There are two outstanding points to address:

  1. Please ensure all new functionality (lines of code may act as a proxy) is under test. This will prevent accidental regression in the future.
  2. Please rework the commit message so it is usable as a release note. Here is an example of a good commit message -- it includes a summary of the change, and the backing motivation; it helps the reader visualize which parts of an apiSpec will generate different OpenAPI output, and how that output has changed.
    This data may not seem important to add in a commit message, since much of the information can be seen in the commit itself, but the release pipeline uses this commit message to create release notes, and this information will be valuable to openapi-generator users.

Once these points are addressed, this code looks ready to ship!

Copy link
Contributor

Choose a reason for hiding this comment

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

Great work in this file, @youngjungithub! Will you please adding any test cases necessary to ensure every custom mapping is under test?

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.

2 participants