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

request config revamp #1520

Merged
merged 34 commits into from Apr 27, 2022
Merged

request config revamp #1520

merged 34 commits into from Apr 27, 2022

Conversation

baywet
Copy link
Member

@baywet baywet commented Apr 13, 2022

Fixes #1494

generation diff

microsoft/kiota-samples#625

Todo

  • bump java abstractions and http

@baywet baywet added generator Issues or improvements relater to generation capabilities. PHP CLI Work to support generating CLIs with Kiota Java TypeScript Pull requests that update Javascript code CSharp Pull requests that update .net code Go Python Ruby area: Swift Tracks work for the swift language support labels Apr 13, 2022
@baywet baywet self-assigned this Apr 13, 2022
@baywet baywet force-pushed the feature/request-options branch 2 times, most recently from c5d6e41 to 79af68b Compare April 19, 2022 14:29
@baywet
Copy link
Member Author

baywet commented Apr 19, 2022

@andrueastman as I suspected I was able to go through Go but not typescript. Thank you for taking this over the finish line while I'm gone.

Here is a high level check-list to help out:

  • add a pull request to kiota-typescript for the headers/options/query parameters vanity methods (some might already be there, and you can look at the PR for go/java/dotnet as a source of inspiration).
  • update the typescript sample in the sample PR.
  • duplicate the unit test AddsMethodsOverloads in the java refiner test to go, adapt it. (sorry meant to do it, ran out of time)
  • add a changelog entry.
  • sonarcloud recommendations.
  • validate it works end to end.
  • once merged, the workflow for java http will fail, just wait for abstractions to go through, and restart it.
  • once merged, create an issue for each of php/python/cli to look at those changes and update their code base.
  • once merged, log an issue in go/dotnet/java/typescript sdks repos to update the snippet generation logic in devx API.

I think that's about it!

@andrueastman
Copy link
Member

No worries. Thanks for the tips @baywet

@andrueastman
Copy link
Member

rosoft/kiota-typescript#146 (comment)

This comment would be applicable to other languages as well

microsoft/kiota-typescript#146 (comment)

cc @nikithauc

@andrueastman
Copy link
Member

Any chance I can get a second review? @MIchaelMainer @zengin @nikithauc @calebkiage

Copy link
Contributor

@zengin zengin left a comment

Choose a reason for hiding this comment

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

LGTM, I left a couple of minor perf comments.

src/Kiota.Builder/Refiners/GoRefiner.cs Outdated Show resolved Hide resolved
src/Kiota.Builder/Refiners/GoRefiner.cs Outdated Show resolved Hide resolved
andrueastman and others added 2 commits April 25, 2022 10:24
Co-authored-by: Mustafa Zengin <muzengin@microsoft.com>
@andrueastman
Copy link
Member

Thanks so much for taking a look @zengin.
The latest update should have the suggestions applied.

@sonarcloud
Copy link

sonarcloud bot commented Apr 25, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

86.5% 86.5% Coverage
0.0% 0.0% Duplication

@baywet
Copy link
Member Author

baywet commented May 2, 2022

Thanks everyone for pushing this across the finish line while I was gone! 🥰

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Swift Tracks work for the swift language support CLI Work to support generating CLIs with Kiota CSharp Pull requests that update .net code generator Issues or improvements relater to generation capabilities. Go Java PHP Python Ruby TypeScript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

api surface revamp for query parameters
5 participants