Skip to content

ClientMethodTemplate: Remove method parameters that can be obtained from other parameters and Improve ClientType to WireType function #7483

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

Conversation

anuchandy
Copy link
Member

@anuchandy anuchandy commented May 27, 2025

  1. In ClientMethodTemplate (and its extended types) methods, there is no need to pass ProxyMethod as an explicit argument, since it can be obtained from ClientMethod argument.
  2. Similarly, no need to provide List<ProxyMethodParameter> explicitly when it can be obtained from ClientMethod ::ProxyMethod::parameters
  3. The function convertClientTypesToWireTypes has been simplified and updated to be extensible for future prs that aim to simplify inherited templates such as client core.
  4. Few other minor cleanup for readability.

AutoRest validation pr.

@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:java Issue for the Java client emitter: @typespec/http-client-java label May 27, 2025
Copy link
Contributor

No changes needing a change description found.

@anuchandy anuchandy self-assigned this May 27, 2025
@azure-sdk
Copy link
Collaborator

azure-sdk commented May 27, 2025

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

@anuchandy anuchandy marked this pull request as draft May 28, 2025 02:24
@anuchandy anuchandy force-pushed the clientMethodTemplate-minor-cleanup branch from d8a2342 to e73a408 Compare May 28, 2025 20:13
@anuchandy anuchandy changed the title ClientMethodTemplate: Remove method parameters that can be obtained from other parameters ClientMethodTemplate: Remove method parameters that can be obtained from other parameters and Improve ClientType to WireType function May 29, 2025
@anuchandy anuchandy marked this pull request as ready for review May 29, 2025 00:15
@anuchandy anuchandy force-pushed the clientMethodTemplate-minor-cleanup branch from 7912cb5 to cff5f07 Compare May 29, 2025 00:33
@haolingdong-msft
Copy link
Member

The changes look good to me.
Good to see we have autorest validation pipeline now. Would like to learn more about working with autorest core submodule(we had submodule before I leave, but at that time, we will just sync the code from typespec repo to autorest.java's core module in local before doing release). Got below questions:

  • My understanding is this pipeline is to run the tests (e.g. vanilla test, fluent test, typespec test) and make sure those are not broken by this pr's change. But I saw it is triggerred on main branch in autorest.java, so it does not include this pr's change, I'm a bit confused on what is this pipeline validating?
  • Is the pipeline triggered manually and when do we trigger it?
  • What is the current process of updating codegen?

@XiaofeiCao
Copy link
Member

XiaofeiCao commented May 29, 2025

Hi @haolingdong-msft , from my understanding:

  • My understanding is this pipeline is to run the tests (e.g. vanilla test, fluent test, typespec test) and make sure those are not broken by this pr's change. But I saw it is triggerred on main branch in autorest.java, so it does not include this pr's change, I'm a bit confused on what is this pipeline validating?

There is a submodule commitId to set core to the latest commit ID of your PR's branch. This is equivalent to checking-out the branch under core when doing autorest.java PR.
image

  • Is the pipeline triggered manually and when do we trigger it?

Currently we trigger it manually. For validation on Swagger part, it's more convenient than preparing a PR in autorest.java repository.

  • What is the current process of updating codegen?

I believe this part hasn't been changed.

Weidong and Anu may provide more context on this.

@weidongxu-microsoft
Copy link
Contributor

To Haoling, the verification pipeline is mostly for convenience. It takes a commitId or a branch.

For PR that we do need a new release, or need version bump on dependencies, or sync of tests, one probably would just go there and create a PR on autorest.java

But for task like refactor, that pipeline could be a help.

@anuchandy anuchandy enabled auto-merge May 29, 2025 14:50
@anuchandy anuchandy added this pull request to the merge queue May 29, 2025
Merged via the queue into microsoft:main with commit 91b1180 May 29, 2025
25 checks passed
@anuchandy anuchandy deleted the clientMethodTemplate-minor-cleanup branch May 29, 2025 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emitter:client:java Issue for the Java client emitter: @typespec/http-client-java
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants