Skip to content

Proposal: Change feed metadata to use attribute level json converter#5466

Closed
yash2710 wants to merge 21 commits into
masterfrom
users/trivediyash/CFPDeleteMetadata
Closed

Proposal: Change feed metadata to use attribute level json converter#5466
yash2710 wants to merge 21 commits into
masterfrom
users/trivediyash/CFPDeleteMetadata

Conversation

@yash2710
Copy link
Copy Markdown
Contributor

Adding on to #5191, the proposal is to use JsonProperty and JsonConverter at attribute level instead of class level.

This PR includes all changes from #5191 and the additional changes.

Updates tests to reflect that the previous image might not be available for delete operations

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Please follow the required format: "[Internal] Category: (Adds|Fixes|Refactors|Removes) Description"

Internal should be used for PRs that have no customer impact. This flag is used to help generate the changelog to know which PRs should be included. Examples:
Diagnostics: Adds GetElapsedClientLatency to CosmosDiagnostics
PartitionKey: Fixes null reference when using default(PartitionKey)
[v4] Client Encryption: Refactors code to external project
[Internal] Query: Adds code generator for CosmosNumbers for easy additions in the future.

/// Applicable for delete operations only, otherwise null.
/// The id of the previous item version.
/// </summary>
[JsonProperty(PropertyName = ChangeFeedMetadataFields.Id, NullValueHandling = NullValueHandling.Ignore)]
Copy link
Copy Markdown
Member

@kirankumarkolli kirankumarkolli Oct 30, 2025

Choose a reason for hiding this comment

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

Its only addresses for Newtonsoft right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Closed offline to use a different approach

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants