envoyproxy / java-control-plane

Java implementation of an Envoy gRPC control plane
Apache License 2.0
293 stars 136 forks source link

Remove deprecated api v2 #241

Closed Automaat closed 2 years ago

codecov-commenter commented 2 years ago

Codecov Report

Merging #241 (e7868b7) into main (e5b9700) will increase coverage by 7.52%. The diff coverage is 97.54%.

@@             Coverage Diff              @@
##               main     #241      +/-   ##
============================================
+ Coverage     82.48%   90.01%   +7.52%     
+ Complexity      275      225      -50     
============================================
  Files            31       28       -3     
  Lines           982      721     -261     
  Branches         78       57      -21     
============================================
- Hits            810      649     -161     
+ Misses          137       49      -88     
+ Partials         35       23      -12     
Impacted Files Coverage Δ
.../controlplane/server/DiscoveryServerCallbacks.java 25.00% <ø> (+5.00%) :arrow_up:
...ne/server/callback/SnapshotCollectingCallback.java 95.55% <ø> (+5.97%) :arrow_up:
...va/io/envoyproxy/controlplane/cache/Resources.java 76.62% <80.00%> (+4.10%) :arrow_up:
.../io/envoyproxy/controlplane/cache/SimpleCache.java 82.73% <100.00%> (-0.25%) :arrow_down:
...o/envoyproxy/controlplane/cache/TestResources.java 100.00% <100.00%> (+12.10%) :arrow_up:
...a/io/envoyproxy/controlplane/cache/XdsRequest.java 100.00% <100.00%> (+19.04%) :arrow_up:
.../io/envoyproxy/controlplane/cache/v3/Snapshot.java 90.38% <100.00%> (+0.38%) :arrow_up:
...lane/server/AdsDiscoveryRequestStreamObserver.java 100.00% <100.00%> (+9.37%) :arrow_up:
...ver/serializer/CachedProtoResourcesSerializer.java 75.00% <100.00%> (-10.72%) :arrow_down:
...er/serializer/DefaultProtoResourcesSerializer.java 100.00% <100.00%> (+15.38%) :arrow_up:
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e5b9700...e7868b7. Read the comment docs.

Automaat commented 2 years ago

I've tested this change on our Allegro environments. It all looks fine. We could merge it already. I am waiting for review from outside of our organisation. @rulex123 Could you look at it?

rulex123 commented 2 years ago

LGTM in general, but there are still a couple of open questions if you could PTAL (I pinged you directly on the specific comments)