fabric8-maven-plugin
fabric8-maven-plugin copied to clipboard
feat(#937): Overrides environment variables when defined in resources
Fix #937 Still no tests or applied to all enrichers because I am not sure about the approach. I think this is the cleanest way of doing it, but waiting for some feedback.
Codecov Report
Merging #1434 into master will increase coverage by
0.32%
. The diff coverage is37.14%
.
@@ Coverage Diff @@
## master #1434 +/- ##
============================================
+ Coverage 34.63% 34.96% +0.32%
- Complexity 1045 1052 +7
============================================
Files 172 171 -1
Lines 9553 9536 -17
Branches 1635 1620 -15
============================================
+ Hits 3309 3334 +25
+ Misses 5869 5839 -30
+ Partials 375 363 -12
well actually this comes of a conversation we had about allowing enrichers to choose if they want to use it or not.
Missatge de Roland Huß [email protected] del dia dj., 13 de des. 2018 a les 14:43:
@rhuss commented on this pull request.
In enricher/standard/src/main/java/io/fabric8/maven/enricher/standard/DependencyEnricher.java https://github.com/fabric8io/fabric8-maven-plugin/pull/1434#discussion_r241403146 :
@@ -119,6 +120,12 @@ private void addArtifactsWithYaml(Set<URL> artifactSet, String dependencyYaml) {
@Override public void adapt(final KubernetesListBuilder builder) {
getConfiguration().getResource().ifPresent(resourceConfig -> {
overrideEnvironmentVariables(builder, resourceConfig.getEnv()
.orElse(new HashMap<>()));
});
Really not sure why we need to duplicate that piece of code in every enricher. For me that looks like a smell.
Whats about a post processing step outside of all enricher, which just overrides the environment variables added by the enricher chain ?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fabric8io/fabric8-maven-plugin/pull/1434#pullrequestreview-184665084, or mute the thread https://github.com/notifications/unsubscribe-auth/ABcmYddBB-BaD32CDp5BSYPnTMFONNDmks5u4lltgaJpZM4YdfqU .
-- +----------------------------------------------------------+ Alex Soto Bueno - Computer Engineer www.lordofthejars.com +----------------------------------------------------------+
@lordofthejars yeah, I remember darkly. Do you have an example for an enricher who does the environment differently ? Not sure anymore whether we should guarantee a consistent behaviour over all enrichers or not.
Test are failing because of a missing JMockit dependency. @lordofthejars could you please have a look ?