-
Notifications
You must be signed in to change notification settings - Fork 43
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
Apply @Incubating
annotations based on the incubating
endpoint tag
#1150
Conversation
Generate changelog in
|
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.
Overall this looks like exactly what we're looking for. Just a couple questions about how to make sure it ties together well for other generators
@@ -24,6 +25,7 @@ public interface TestService { | |||
* Returns a mapping from file system id to backing file system configuration. | |||
* @apiNote {@code GET /catalog/fileSystems} | |||
*/ | |||
@Beta |
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.
Would it make sense to add a comment to this to explain that this corresponds to "Unstable"? Otherwise, I'd be fine with calling the APIs "Beta". I'm also curious what options we have for typescript, to ensure that they all make sense together.
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 not sure how this will work in typescript, but I imagine there are similar extension points. I don't think endpoint tags have been implemented in any in conjure-typescript yet.
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.
deprecated recently got added as a jsdoc: palantir/conjure-typescript#149
I went through the list of jsdoc tags (https://jsdoc.app/), but didn't see any that looked quite right. I'm not familiar with the ecosystem to know if there are other standards that could be leveraged besides jsdoc, but the support for jsdoc's @Deprecated
only arrived in vscode in August (https://code.visualstudio.com/updates/v1_49#_deprecated-tag-support-for-javascript-and-typescript), so I don't have a lot of confidence there are good fits available.
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.
Custom vscode plugins might be an option
conjure-java-core/src/main/java/com/palantir/conjure/java/ConjureAnnotations.java
Outdated
Show resolved
Hide resolved
Using the existing guava one is definitely neat, but I think it does come with the tradeoff that since so many (well used) guava APIs are already marked 'beta', it might be hard for a project to 'lock in' this check, if they wanted to have a stamp of approval that says "I don't use any beta endpoints"? A little bit like when people are evaluating a rust crate, they're often interested to know if it contains any 'unsafe', as this might be a source of spicy bugs in the future |
@Beta
annotations based on the unstable
endpoint tag@Incubating
annotations based on the incubating
endpoint tag
Related: palantir/gradle-baseline#1529 |
Released 5.34.0 |
Quick draft of what we discussed, the guava
@Beta
annotation turns invocations yellow in idea based on UnstableApiUsageInspection.kt#35==COMMIT_MSG==
Apply guava
@Beta
annotations based on theunstable
endpoint tag==COMMIT_MSG==
Considerations:
unstable
. Would we preferincubating
, or something different?@Beta
annotation. We can define our own inconjure-lib
and update our idea configuration to apply it to the UnstableApiUsageInspection list (greater complexity, must work for both idea gradle integration and the idea ipr).