-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
[FLINK-36207][build] Disables deprecated API from japicmp #25285
Conversation
<exclude>@org.apache.flink.annotation.Experimental</exclude> | ||
<exclude>@org.apache.flink.annotation.PublicEvolving</exclude> | ||
<exclude>@org.apache.flink.annotation.Internal</exclude> | ||
<!-- MARKER: start exclusions; these will be wiped by tools/releasing/update_japicmp_configuration.sh --> | ||
<!-- Mark these 2 methods to @Internal. Tracked under FLINK-34130, should be removed in 2.0 --> | ||
<exclude>org.apache.flink.configuration.Configuration#getBytes(java.lang.String,byte[])</exclude> | ||
<exclude>org.apache.flink.configuration.Configuration#setBytes(java.lang.String,byte[])</exclude> | ||
<!-- FLINK-35886: WatermarksWithIdleness constructor was marked as deprecated --> | ||
<exclude>org.apache.flink.api.common.eventtime.WatermarksWithIdleness</exclude> |
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 struggled to get my head around why this exclusion was necessary. It's a change that was introduced in 2.0 (see FLINK-35886) which deprecated the constructor.
My reasoning now is that the reference data (that was generated as part of the 1.20 release branch creation) didn't include that deprecation, yet. Excluding deprecated APIs now makes them be ignored in the check, i.e. the API disappears from the compare-to set (master HEAD) but is still present in the base set (1.20.0). 🤔
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.
Thanks, @XComp. LGTM. Please make sure to resolve the conflicts before merging.
The japicmp plugin doesn't seem to work well with Scala annotations (more specifically @scala.deprecated). But we're planning to remove the Scala code as part of the 2.0 release, anyway. Hence, excluding all *scala files should be good enough.
@flinkbot run azure |
What is the purpose of the change
The japicmp plugin doesn't seem to work well with Scala annotations (more specifically @scala.deprecated). But we're planning to remove the Scala code as part of the 2.0 release, anyway. Hence, excluding all *scala files should be good enough.
Brief change log
Verifying this change
Check the corresponding branches the workflows ran on to see the changes.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation