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

Create types for untyped nodes #177

Merged
merged 12 commits into from
Mar 20, 2024

Conversation

MartinM85
Copy link
Contributor

Closes #175

@MartinM85
Copy link
Contributor Author

I've made changes in kiota-serialization-json-dotnet, but need to reference kiota-abstractions-dotnet 1.7.6 before creating a pull request.

Suppose that at the end, the SDK generator will generate something like this

    public class WorkbookFilterCriteria : IAdditionalDataHolder, IBackedModel, IParsable {
...
        public UntypedNode? Values {
            get { return BackingStore?.Get<UntypedNode?>("values"); }
            set { BackingStore?.Set("values", value); }
        }
...
        public virtual IDictionary<string, Action<IParseNode>> GetFieldDeserializers() {
            return new Dictionary<string, Action<IParseNode>> {
...
                {"values", n => { Values = n.GetUntypedValue(); } },
            };
        }
        public virtual void Serialize(ISerializationWriter writer) {
            _ = writer ?? throw new ArgumentNullException(nameof(writer));
...
            writer.WriteUntypedValue("values", Values);
            writer.WriteAdditionalData(AdditionalData);
        }
    }

instead of the current:

  public class WorkbookFilterCriteria : IAdditionalDataHolder, IBackedModel, IParsable {
...

        public Json? Values {
            get { return BackingStore?.Get<Json?>("values"); }
            set { BackingStore?.Set("values", value); }
        }
...
        public virtual IDictionary<string, Action<IParseNode>> GetFieldDeserializers() {
            return new Dictionary<string, Action<IParseNode>> {
...
                {"values", n => { Values = n.GetObjectValue<Json>(Json.CreateFromDiscriminatorValue); } },
            };
        }
        public virtual void Serialize(ISerializationWriter writer) {
            _ = writer ?? throw new ArgumentNullException(nameof(writer));
...
            writer.WriteObjectValue<Json>("values", Values);
            writer.WriteAdditionalData(AdditionalData);
        }
    }

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, a couple of remarks. I think you can implement the POC with a local reference for the json serialization and see how that turns out.

src/serialization/UntypedArray.cs Outdated Show resolved Hide resolved
src/serialization/IParseNode.cs Outdated Show resolved Hide resolved
@MartinM85 MartinM85 requested a review from baywet January 17, 2024 06:50
src/serialization/UntypedNode.cs Outdated Show resolved Hide resolved
src/serialization/UntypedNode.cs Outdated Show resolved Hide resolved
@MartinM85 MartinM85 requested a review from baywet January 18, 2024 20:30
Copy link
Member

@andrueastman andrueastman left a comment

Choose a reason for hiding this comment

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

Just a comment on this side.
Otherwise, we need a version bump to 1.8.0 as well as a changelog entry.

src/serialization/UntypedNode.cs Outdated Show resolved Hide resolved
src/serialization/UntypedLong.cs Outdated Show resolved Hide resolved
@MartinM85 MartinM85 requested a review from baywet January 31, 2024 08:42

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Copy link
Contributor

github-actions bot commented Mar 6, 2024

Conflicts have been resolved. A maintainer will take a look shortly.

baywet
baywet previously approved these changes Mar 6, 2024
Copy link
Contributor

This pull request has conflicting changes, the author must resolve the conflicts before this pull request can be merged.

# Conflicts:
#	CHANGELOG.md
#	src/Microsoft.Kiota.Abstractions.csproj
@baywet
Copy link
Member

baywet commented Mar 19, 2024

Just as an update to anyone watching this: we're finalizing the work on the other main languages before we merge everything in. Thank you for your patience :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Create types for untyped nodes
3 participants