-
Notifications
You must be signed in to change notification settings - Fork 736
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
Implement BetaApi annotation for hub4j sdk #1002
Conversation
@@ -78,7 +78,7 @@ public String getName() { | |||
* | |||
* @return true if the push to this branch is restricted via branch protection. | |||
*/ | |||
@Preview | |||
@Preview(Previews.LUKE_CAGE) |
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.
This was a miss from the preview PR caught by the new arch tests
@@ -121,7 +121,7 @@ public void update(String body) throws IOException { | |||
this.body = body; | |||
} | |||
|
|||
@Preview | |||
@Preview(SQUIRREL_GIRL) |
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.
This was a miss from the preview PR caught by the new arch tests
@@ -260,7 +260,6 @@ public GHAsset uploadAsset(String filename, InputStream stream, String contentTy | |||
* existing logic in place for backwards compatibility. | |||
*/ | |||
@Deprecated | |||
@Preview | |||
public List<GHAsset> assets() { |
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 was unable to find anything in the Github API documentation that confirms this is a preview API.
https://developer.github.com/v3/repos/releases/#list-release-assets
@@ -19,6 +23,17 @@ | |||
.withImportOption(new ImportOption.DoNotIncludeJars()) | |||
.importPackages("org.kohsuke.github"); | |||
|
|||
private static final DescribedPredicate<JavaAnnotation<?>> previewAnnotationWithNoMediaType = new DescribedPredicate<JavaAnnotation<?>>( |
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.
This predicate ensures that no usages of the preview annotation will accept an empty array for the value param.
@@ -42,8 +42,6 @@ public String getEmail() { | |||
* | |||
* @return GitHub username | |||
*/ | |||
@Preview | |||
@Deprecated | |||
@CheckForNull |
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.
This method was not labeled properly. The field itself is not part of a preview API. It is only optionally present depending on the source of the data. We've already indicated the potential lack of presence with the @CheckForNull
annotation.
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.
This looks great.
Description
This PR introduces a
@BetaApi
annotation to help differentiate Github@Preview
APIs from hub4j beta APIs. The reasoning behind this change is based on the findings outlined in #1001. With these changes, the preview media type is now a hard requirement. Architecture tests have been added to ensure beta APIs are annotated as deprecated and that usages of preview do not allow empty arrays.Before submitting a PR:
We love getting PRs, but we hate asking people for the same basic changes every time.
master
. Create your PR from that branch.mvn clean compile
locally. This may reformat your code, commit those changes.mvn -D enable-ci clean install site
locally. If this command doesn't succeed, your change will not pass CI.When creating a PR: