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

Make Parameter & PropertyDeclaration Classes Instead of Records #3450

Merged
merged 10 commits into from
May 30, 2024

Conversation

jorgerangel-msft
Copy link
Contributor

@jorgerangel-msft jorgerangel-msft commented May 24, 2024

This PR updates the Parameter and PropertyDeclaration types by converted them into classes and adds constructors to build both types from an InputModelProperty.

@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label May 24, 2024
@azure-sdk
Copy link
Collaborator

azure-sdk commented May 24, 2024

API change check

API changes are not detected in this pull request.

@ArcturusZhang
Copy link
Member

So I do not think we want to close the Parameter, FieldDeclaration and PropertyDeclaration classes for users and only leave a few entry points for them, as far as I could read from our comments, these entry points are:

  • from input parameter/property
  • from a known list of parameters (how about field and property?)

These classes are defining a member in CSharp language therefore I think when our plugin writer is writing something, they might always need to write something that does not come from the spec, therefore they might need to construct their own parameter or field or property.
For instance in the patch implementation, our current design introduces quite a few extra fields and properties, for example the backing field of the property, the IsChanged property for the model itself, some of them do not come from a input type.
Therefore I think we should keep those ctors to construct a parameter/field/property from scratch public.

Also I think it makes more sense to construct a parameter/field/property from input if those are not ctors but static methods.

@jorgerangel-msft jorgerangel-msft requested a review from m-nash May 30, 2024 16:33
@jorgerangel-msft jorgerangel-msft added this pull request to the merge queue May 30, 2024
Merged via the queue into microsoft:main with commit 4d1f0a1 May 30, 2024
21 checks passed
@jorgerangel-msft jorgerangel-msft deleted the refactor-properties branch May 30, 2024 22:44
github-merge-queue bot pushed a commit that referenced this pull request Jun 3, 2024
Follow up to #3450. This PR
updates the `FieldDeclaration` from a record to a class to follow the
pattern set by PropertyDeclaration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Moving serialization model constructors
4 participants