fixing #issue872 #1625
fixing #issue872 #1625
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1625 +/- ##
============================================
+ Coverage 34.8% 35.24% +0.44%
- Complexity 1121 1123 +2
============================================
Files 186 186
Lines 10314 10188 -126
Branches 1680 1656 -24
============================================
+ Hits 3590 3591 +1
+ Misses 6302 6174 -128
- Partials 422 423 +1 |
enricher/standard/src/main/java/io/fabric8/maven/enricher/standard/DefaultServiceEnricher.java
Outdated
Show resolved
Hide resolved
enricher/standard/src/main/java/io/fabric8/maven/enricher/standard/DefaultServiceEnricher.java
Outdated
Show resolved
Hide resolved
enricher/standard/src/main/java/io/fabric8/maven/enricher/standard/DefaultServiceEnricher.java
Outdated
Show resolved
Hide resolved
enricher/standard/src/main/java/io/fabric8/maven/enricher/standard/DefaultServiceEnricher.java
Outdated
Show resolved
Hide resolved
enricher/standard/src/main/java/io/fabric8/maven/enricher/standard/DefaultServiceEnricher.java
Show resolved
Hide resolved
| try { | ||
| if (getContext().getProjectClassLoaders().isClassInCompileClasspath(true)) { | ||
| Properties properties = SpringBootUtil.getSpringBootApplicationProperties(getContext().getProjectClassLoaders().getCompileClassLoader()); | ||
| return properties.getProperty("spring.application.name"); |
There was a problem hiding this comment.
@ro14nd @lordofthejars @rohanKanojia can you provide other property names that we should consider for the name ?
- spring.app.name
- app.name
- spring.service.name ?
WDYT ?
There was a problem hiding this comment.
we should stick to standard spring properties, which is spring.application.name in this case. If there are other ways there, then we can add them here as well.
There was a problem hiding this comment.
yeah, that's what I meant..
I think spring.app.name should be considered as well
There was a problem hiding this comment.
@theexplorist can you add a getProperty("spring.app.name") as well? Even in #872 its mentioned spring.app.name .
devang-gaur
added
the
pr/changelog-entry-please
|
@dev-gaur : Can you please take a look if there is anything pending from your side? Otherwise, can we proceed to merge this PR? |
I will update the PR with requested changes by today so it can be good to merge. |
updated changes
rohanKanojia
removed
the
pr/changelog-entry-please
| try { | ||
| if (getContext().getProjectClassLoaders().isClassInCompileClasspath(true)) { | ||
| Properties properties = SpringBootUtil.getSpringBootApplicationProperties(getContext().getProjectClassLoaders().getCompileClassLoader()); | ||
| return properties.getProperty("spring.application.name"); |
There was a problem hiding this comment.
@theexplorist can you add a getProperty("spring.app.name") as well? Even in #872 its mentioned spring.app.name .
| } | ||
|
|
||
| private String getServiceName() { | ||
| if (getAppName() != null) { |
There was a problem hiding this comment.
Can you skip calling getAppName twice? Do something like:
String appName = getAppName();
if (appName!=null) {
return appName;
} else {
....
}
After that, I think we are ready to merge.
fixes #872