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

Allow excluding fields from interfaces #1975

Merged
merged 6 commits into from
Dec 7, 2023

Conversation

kyri-petrou
Copy link
Collaborator

@kyri-petrou kyri-petrou commented Nov 2, 2023

Closes #1829.

@paulpdaniels what are your thoughts on this approach? I thought it might be better to reuse the same annotation and add the option to provide a list of field names that you want to hide from the interface. The change is not binary compatible but it's largely source compatible so users won't need to migrate any code

Alternatively, we could add a separate annotation that is applied on fields if we think it's more appropriate

@paulpdaniels
Copy link
Collaborator

Would there be any value to having it separate? Like any scenarios we might want to allow it standalone? Otherwise I imagine that might have some footgun behavior if someone forgot to add both.

@kyri-petrou
Copy link
Collaborator Author

Would there be any value to having it separate? Like any scenarios we might want to allow it standalone? Otherwise I imagine that might have some footgun behavior if someone forgot to add both.

I can't think of any benefit in having it separate other than avoiding the binary incompatible change. Just thought to point it out just in case there was something I might have missed

# Conflicts:
#	core/src/main/scala-2/caliban/schema/SchemaDerivation.scala
#	core/src/main/scala-3/caliban/schema/SchemaDerivation.scala
@kyri-petrou
Copy link
Collaborator Author

@ghostdogpr If I merge this in the next release will need to be 2.5.0 due since there are binary incompatible changes in this PR. Are we okay with that?

@ghostdogpr
Copy link
Owner

In theory yes. I'd like to wait for the zio-http RC4 release to do all the breaking changes at once (with also the Play upgrade) so I hope it won't take too long. If we merge this we'll have to wait for the next release, but we can always do bug fixes in a branch if we really need to.

@kyri-petrou
Copy link
Collaborator Author

In theory yes. I'd like to wait for the zio-http RC4 release to do all the breaking changes at once (with also the Play upgrade) so I hope it won't take too long. If we merge this we'll have to wait for the next release, but we can always do bug fixes in a branch if we really need to.

Ok sounds good to me 👍

@kyri-petrou
Copy link
Collaborator Author

By the way I wonder whether we should have a Breaking change (or similar) label just so that we know which PRs contain breaking changes without having to look through the code

@ghostdogpr
Copy link
Owner

ghostdogpr commented Nov 19, 2023

We could add that label 👍

@kyri-petrou kyri-petrou added the breaking change The PR contains binary incompatible changes label Nov 19, 2023
@kyri-petrou kyri-petrou merged commit 2a9d4fb into series/2.x Dec 7, 2023
10 checks passed
@kyri-petrou kyri-petrou deleted the exclude-fields-from-interfaces branch December 7, 2023 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change The PR contains binary incompatible changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support a way to hide fields from interfaces
3 participants