Skip to content

Documentation of Configuration/ConfigurationParameter for consolidated TypeScript generator #10283

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

Merged
merged 7 commits into from
Jan 11, 2023

Conversation

TiFu
Copy link
Contributor

@TiFu TiFu commented Aug 29, 2021

As requested by @rgomezp in #6341, I have added a bit more elaborate comments on the ConfigurationParameters and Configuration objects.

Just an initial draft for now, but happy to already get some feedback on how we can further improve the documentation to make it more user-friendly.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master (5.3.0), 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

CC @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02)

@rgomezp
Copy link

rgomezp commented Aug 30, 2021

This is great @TiFu thank you.

Two comments:

  1. You refer several times to the "spec", but where is this spec? As someone who is new to this library, I struggled to become familiar with the API for this generator since the docs didn't have any info on the specific configuration API.
  2. Could some of this info also be added to the documentation? Regardless, these new type annotations certainly help loads.

@bodograumann
Copy link
Contributor

The spec is the OpenAPI file that you use to generate the code, @rgomezp .
Maybe speaking about the OpenAPI definition would be more on-point, because spec can also mean the OpenAPI 2/3 specification itself.

const config = your_api.createConfiguration(configurationParameters);

// Use configuration with your_api
const api = new your_api.PetApi(config);

Choose a reason for hiding this comment

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

Would be great to also see how you would make a call off of this api instance.
It seems like we'd do something like api.getUsers(), but I'm getting tripped up by POST requests.

It seems like you'd do something like:

// Seems like object arguments need to be namespaced, and that's based on the model name?
// If there's an option to not-namespace params I'd love to know it
api.createUser({createUserRequest: { email: 'email', password: 'password'}})

This seems to work for me if the SDK is making a call from one localhost port to another, but in my prod env it strips out the request payload. I imagine that's because I'm missing some kind of config option, and seeing how you do this would be super helpful information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added another couple lines with an example in the latest commit. Hope that explains it a bit more. There's also some examples in the tests folder under samples/openapi3/client/petstore/typescript/ - hope this helps with debugging your problem.

@TiFu TiFu force-pushed the typescript-documentation branch from 943a3dd to be67222 Compare January 10, 2023 10:52
@TiFu
Copy link
Contributor Author

TiFu commented Jan 11, 2023

Failures are unrelated.

Given that this only touches files in the consolidated typescript generator & only changes comments, will go ahead and merge this.

@TiFu TiFu changed the title [Draft] Documentation of Configuration/ConfigurationParameter for consolidated TypeScript generator Documentation of Configuration/ConfigurationParameter for consolidated TypeScript generator Jan 11, 2023
@TiFu TiFu merged commit a31b5b1 into master Jan 11, 2023
@wing328 wing328 added this to the 6.3.0 milestone Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants