accumulo icon indicating copy to clipboard operation
accumulo copied to clipboard

Create a QA check for GitHub Actions that checks for breaking API changes

Open ctubbsii opened this issue 4 years ago • 3 comments

Is your feature request related to a problem? Please describe. Sometimes, we unintentionally break API. It would be nice if we could detect this in a PR.

Describe the solution you'd like A new matrix line in our QA / CI checks in GitHub Actions could be added to check our public API classes to see if they have had a breaking change (change of method signature or removal of method). We could also detect API additions. It would be okay for this check to fail, if the change is actually intended. But, having this check fail when such changes do occur in a PR will bring our attention to changes we might not have intended to make.

Describe alternatives you've considered Catch it in code review manually (what we do now) or check periodically (what we tend to do at release time).

Additional context N/A

ctubbsii avatar Apr 21 '21 20:04 ctubbsii

What command do we run at release time to check for API breakage? Just maven's apilyzer plugin?

Manno15 avatar Aug 13 '21 16:08 Manno15

We use the apilyzer-maven-plugin. Check out https://accumulo.apache.org/blog/2019/11/04/checkstyle-import-control.html

milleruntime avatar Aug 13 '21 17:08 milleruntime

This task is basically to highlight whenever we change the API in a potentially breaking way. It's not necessarily intended to restrict the imports or to analyze the API classes, but just to highlight that the API changed in some way. I hadn't put a lot of thought into how this would work, but it could theoretically work by analyzing the diff against the base branch to see whether any public API methods have been added, deleted, or modified in public API classes. The intent wasn't to identify a PR that should not be merged, but just to fail the task to highlight that we should look more closely at the API implications. The task could be called "No API changes" or similar, so that a failure would simply imply that there were API changes.

ctubbsii avatar Aug 17 '21 22:08 ctubbsii