Skip to content

Conversation

@vdorokhin
Copy link
Contributor

fixes #441

Copy link
Member

@nrwiersma nrwiersma left a comment

Choose a reason for hiding this comment

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

This change breaks backwards compatibility. While this change may suit your needs, it is common for people to use canonical forms of the schema.

@vdorokhin
Copy link
Contributor Author

This change breaks backwards compatibility. While this change may suit your needs, it is common for people to use canonical forms of the schema.

With the current implementation, everyone who used the String() method to retrieve the schema will continue receiving the canonical version of the schema. However, if needed, access to other schema data will also be available. In any case, I’m open to making additional changes. I can add a flag to control this behavior. Would that be a suitable option?

@nrwiersma
Copy link
Member

With the current implementation, everyone who used the String() method to retrieve the schema will continue receiving the canonical version of the schema. However, if needed, access to other schema data will also be available. In any case, I’m open to making additional changes.

While true, you are changing the default behaviour of the application. It is reasonable to assume that the same inputs by enlarge produce the same outputs.

I suggest an option to store the "full" output, perhaps WithFullSchema.

@vdorokhin
Copy link
Contributor Author

I added the -fullschema flag to enable this behavior. In the tests, I used it in combination with -encoders for easier verification

@vdorokhin vdorokhin requested a review from nrwiersma September 10, 2024 09:13
Copy link
Member

@nrwiersma nrwiersma left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 Thanks for the contribution.

@nrwiersma nrwiersma merged commit a6c674d into hamba:main Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Keep a detailed version of schema in case of generation with encoders flag

2 participants