Skip to content
This repository has been archived by the owner on Jun 19, 2024. It is now read-only.

refactor(enricher): Cleanup up fabric8 specific enrichers #1376

Merged
merged 2 commits into from
Sep 27, 2018

Conversation

rhuss
Copy link
Contributor

@rhuss rhuss commented Sep 11, 2018

Enrichers which are specific to the fabric8 platform (CD + UI) has been
moved into a "deprecated" enricher module. This can be kept as reference
or later easily removed. No enricher in this module is include in any
default profile and hence is disabled.

The enrichers that were disabled that way are:

  • CdEnricher
  • DocLinkEnricher
  • GrafanaLinkEnricher
  • IconEnricher

Some other refactoring consolidates on the naming of enrichers, so that
everything with a f8- prefix currently is about auto detection of
specific tech stacks (like specialized healthchecks or the prometheus
enricher)

All other are in the "standard" module and have a "fmp" prefix.

The MavenScmEnricher has been simplified a bit to remove duplicated SCM
information ("connection" and "developerConnection")

All integration tests have been adapted accordingly.

@rhuss
Copy link
Contributor Author

rhuss commented Sep 11, 2018

There is still one thing which I would like to improve in this PR: Currently in the "fabric8" enricher module, there are mostly healthcheck enrichers, one for each tech stack.

I'd like to move these health check enrichers to something more general like e.g. a "SpringBootEnricher" which is responsible for all Spring Boot specific enrichment (which currently is only for health checks). This would have the benefit to reduce the name of the enricher as it is also used in the configurtion to "spring-boot".

For this to work properly though, we should be able to allow nested configurtion from the very beginning so that a configuration for such an enricher could look like:

<enricher>
  <config>
    <spring-boot>
      <health>
         ...
      </health>
    </spring-boot>
  </config>

</enricher>

This could also map nicely to properties like it is done in the vertx-healtcheck enricher.

See also #1375 for this deep configuration aspect.

Of course, this probably would make sense for genertors, too.

@codecov
Copy link

codecov bot commented Sep 11, 2018

Codecov Report

Merging #1376 into master will increase coverage by 1.7%.
The diff coverage is 17.64%.

@@             Coverage Diff             @@
##             master    #1376     +/-   ##
===========================================
+ Coverage     31.88%   33.58%   +1.7%     
- Complexity     1001     1002      +1     
===========================================
  Files           174      169      -5     
  Lines          9915     9447    -468     
  Branches       1790     1652    -138     
===========================================
+ Hits           3161     3173     +12     
+ Misses         6403     5918    -485     
- Partials        351      356      +5

@rhuss rhuss added the target/4.0 PR for targeted to 4.0.x label Sep 11, 2018
@rhuss
Copy link
Contributor Author

rhuss commented Sep 11, 2018

Fixes #1330

Copy link
Member

@rohanKanojia rohanKanojia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, Thanks 👍

@@ -754,7 +730,7 @@ For example:
<path>/ping</path>
<port>8080</port>
</readiness>
</vertx-health-check>
</f8-healthcheck>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be f8-healthcheck-vertx?

Enrichers which are specific to the fabric8 platform (CD + UI) has been
moved into a "deprecated" enricher module. This can be kept as reference
or later easily removed. No enricher in this module is include in any
default profile and hence is disabled.

The enrichers that were disabled that way are:

* CdEnricher
* DocLinkEnricher
* GrafanaLinkEnricher
* IconEnricher

Some other refactoring consolidates on the naming of enrichers, so that
everything with a `f8-` prefix currently is about auto detection of
specific tech stacks (like specialized healthchecks or the prometheus
enricher)

All other are in the "standard" module and have a "fmp" prefix.

The MavenScmEnricher has been simplified a bit to remove duplicated SCM
information ("connection" and "developerConnection")

All integration tests have been adapted accordingly.
@rhuss rhuss force-pushed the pr/cleanup-f8-enrichers branch from b33867a to 2f4fa46 Compare September 24, 2018 13:21
@rhuss rhuss merged commit c455c58 into fabric8io:master Sep 27, 2018
lordofthejars pushed a commit to lordofthejars/fabric8-maven-plugin that referenced this pull request Jan 17, 2019
…1376)

Enrichers which are specific to the fabric8 platform (CD + UI) has been
moved into a "deprecated" enricher module. This can be kept as reference
or later easily removed. No enricher in this module is include in any
default profile and hence is disabled.

The enrichers that were disabled that way are:

* CdEnricher
* DocLinkEnricher
* GrafanaLinkEnricher
* IconEnricher

Some other refactoring consolidates on the naming of enrichers, so that
everything with a `f8-` prefix currently is about auto detection of
specific tech stacks (like specialized healthchecks or the prometheus
enricher)

All other are in the "standard" module and have a "fmp" prefix.

The MavenScmEnricher has been simplified a bit to remove duplicated SCM
information ("connection" and "developerConnection")

All integration tests have been adapted accordingly.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
target/4.0 PR for targeted to 4.0.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants