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

Add API support for multiple schema extension definitions #1578

Closed
wants to merge 1 commit into from

Conversation

loligans
Copy link

@loligans loligans commented Dec 3, 2022

This PR enables a developer to define attributes that allow multiple schema extensions to be defined.

Fixes #1568

@loligans
Copy link
Author

loligans commented Dec 3, 2022

Would appreciate a review whenever @lahma or @RicoSuter have the time. This isn't a big change but I'd be happy to hear any feedback you might have

namespace NJsonSchema.Annotations
{
/// <summary>Interface to add multiple extension data property to a class or property, implementation needs to inherit from System.Attribute.</summary>
public interface IMultiJsonSchemaExtensionDataAttribute
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not a subject matter expect here (Rico is), but would it make sore sense just to make a breaking change to change the support to have dictionary in original interface so there wouldn't be two ways to achieve the same thing (adding one item can be done via dictionary or via old interface).

Copy link
Author

Choose a reason for hiding this comment

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

this definitely could be done, I took the more conservative approach because I didn't want to break existing functionality. I don't mind refactoring to do what you suggest if that's what is agreed upon

Copy link
Author

Choose a reason for hiding this comment

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

would appreciate a review @RicoSuter when you have some time

@RicoSuter RicoSuter closed this in 8e65d2d Sep 26, 2023
@RicoSuter
Copy link
Owner

v11

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.

Allow defining multiple keys
4 participants