-
Notifications
You must be signed in to change notification settings - Fork 17
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
Mark v0 protos with deprecations #28
Conversation
authzed/api/v0/core.proto
Outdated
@@ -24,6 +27,7 @@ message RelationTuple { | |||
} | |||
|
|||
message ObjectAndRelation { | |||
option deprecated = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Several of the core types (ObjectAndRelation
, RelationRerefence
, and RelationTupleTreeNode
at least) are used in the dispatch api. Marking them deprecated here will (I think?) make our linters complain about using deprecated types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh interesting, I didn't expect the generated deprecated comment in go code to be parsed by the linter. We can remove them for these messages in the interim.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep! Most recently it was some of the grpc options we were using that were deprecated. I looked it up and it's staticcheck that complains: https://staticcheck.io/docs/checks/#SA1019
Other options would be to take these types and copy them into the dispatch API and only mark these v0 versions deprecated, or we can mark these deprecated now and just tell the linter to ignore it with a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we going to block on that? It seems to me like eventually the dispatch API should use its own types or be migrated to the latest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm leaning toward leaving the messages in core un-deprecated. The services are all still annotated and we can avoid having to add ignore comments a bunch of places. We can separately work on copying these into the dispatch API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI did the same for namespace message definitions because they're used in schemadsl
7d93cb8
to
0bdb2e0
Compare
@jzelinskie any other feedback? |
Adds deprecations to v0 proto files using the findings below to provide the most granular notice in generated code.
Result of
deprecated = true
in generated code@deprecated
@deprecated
@deprecated
on file@java.lang.Deprecated
@java.lang.Deprecated
on classes