refactor (jkube-kit) : Remove RemoveBuildAnnotationsEnricher (#1263) #1412
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1412 +/- ##
============================================
- Coverage 51.07% 51.04% -0.03%
+ Complexity 3801 3800 -1
============================================
Files 459 459
Lines 20680 20678 -2
Branches 2804 2804
============================================
- Hits 10562 10555 -7
- Misses 9011 9016 +5
Partials 1107 1107
Continue to review full report at Codecov.
|
rohanKanojia
requested review from
manusa and
sunix
and removed request for
manusa
| public static void removeResourceSourceUrlAnnotation(KubernetesListBuilder builder) { | ||
| builder.accept(new TypedVisitor<ObjectMetaBuilder>() { | ||
| @Override | ||
| public void visit(ObjectMetaBuilder objectMetaBuilder) { | ||
| if (objectMetaBuilder != null) { | ||
| objectMetaBuilder.removeFromAnnotations(Constants.RESOURCE_SOURCE_URL_ANNOTATION); | ||
| } | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
I'm still not sure why we still keep this and the related call from DependencyEnricher.
I understand that this might come from a Dependency JAR, but if the dependency has an aligned version of JKube, then it shouldn't be necessary.
IMHO it might be fine to keep this, but we should open a Deprecation issue for 2.x milestone, and add a @deprecated note+annotation in this method with a link to the issue. WDYT?
There was a problem hiding this comment.
I checked and it looks like this annotation was already marked as deprecated in #1245 :
There was a problem hiding this comment.
Not the annotation, the new method should be marked as deprecated, since we're just adding it temporarily (right?).
Also create the issue to remove this method in 2.0 (plus add it as a comment in the code).
rohanKanojia
force-pushed
the
pr/issue1263
branch
from
7639351
to
c031f40
Compare
rohanKanojia
force-pushed
the
pr/issue1263
branch
2 times, most recently
from
13e2991
to
6bb7fab
Compare
...er/api/src/test/java/org/eclipse/jkube/kit/enricher/api/util/KubernetesResourceUtilTest.java
Outdated
Show resolved
Hide resolved
…-jkube#1263) RemoveBuildAnnotationsEnricher seems to be removing `maven.jkube.io/source-url` annotation which gets added in DependencyEnricher. This annotation doesn't seem to be getting used anywhere else. Delete RemoveBuildAnnotationsEnricher & `maven.jkube.io/source-url` annotation logic inside it to DependencyEnricher Signed-off-by: Rohan Kumar <rohaan@redhat.com>
rohanKanojia
force-pushed
the
pr/issue1263
branch
from
6bb7fab
to
2ba9434
Compare
|
Kudos, SonarCloud Quality Gate passed! |
Description
Fix #1263
Fix #1465
RemoveBuildAnnotationsEnricher seems to be removing
maven.jkube.io/source-urlannotation which gets added inDependencyEnricher. This annotation doesn't seem to be getting used anywhere else.
Delete RemoveBuildAnnotationsEnricher &
maven.jkube.io/source-urlannotation logic inside it to DependencyEnricherNote that annotations being removed from resource integration tests are due to side effects of PodAnnotationEnricher modifying resource in between DependencyEnricher and RemoveBuildAnnotationsEnricher. They were not cleaned up properly in RemoveBuildAnnotationsEnricher. With the removal of RemoveBuildAnnotationsEnricher and
maven.jkube.io/source-urlannotation removed within DependencyEnricher, these annotations are not available to PodAnnotationEnricher like before.See #1263 (comment) for more details.
Signed-off-by: Rohan Kumar rohaan@redhat.com
Type of change
test, version modification, documentation, etc.)
Checklist