reactor/reactor

Have all project declare an explicit Java9 Automatic Module Name

simonbasle opened this issue · 7 comments

simonbasle commented

The Automatic Module feature has 2 modes in terms of how it choses the generated module's name:

  • implicit: the name is derived from the jar filename, which could change eg. if someone imports a renamed jar manually rather than through Maven/Gradle...
  • explicit: the name is declared by the library maintainer in the MANIFEST.MF as an Automatic-Module-Name attribute.

The later solution is more stable, but makes the module name part of the API: this is basically a pledge to use that module name later on when explicit modularization is introduced. Note that changing the module name is not a light thing: every dependent modularized code will need to be updated to requires the new module name.

This epic is two-parts: choosing a name pattern for the projects and making sure no same package is split between two "modules".

simonbasle commented

Choosing an Automatic-Module-Name

The derived name from the standard Maven naming of the artifacts is in the form reactor.core/reactor.test/reactor.netty, with a few simpler ones like blockhound...

While this could be enough (this is what the Spring Framework is using for instance), there is a need for the module name to be globally unique, and reactor.core might be a touch too generic...

Proposal: use io.projectreactor.XXX as a fuller prefix (even though our packages are shorter, which might be a separate issue). See comment below for a complete list of proposed modules.

  • 👍1
simonbasle commented

Split Packages

The JPMS enforces that no two modules can export the same package. This is for classes that are directly in said packages, so reactor.adapter vs reactor.adapter.akka is fine.

However there are at least a couple of projects where there is room for improvement in terms of packages: a more specific "root" package would be better:

  • Blockhound has some classes directly in the reactor package, could be moved to reactor.blockhound (reactor/BlockHound#25)
  • deferred to 4.0 reactor-extra has a bunch of low-level packages under reactor.* which increases chances of clashes, could all be moved under reactor.extra.* (reactor/reactor-addons#203)
  • ...
simonbasle commented

Module Names Proposal 1

*Rejected*
  • reactor-core:
    • io.projectreactor.core
    • io.projectreactor.test
  • reactor-addons:
    • io.projectreactor.adapter (exports packages reactor.adapter.akka and reactor.adapter.rxjava, NOT reactor.adapter)
    • io.projectreactor.extra
    • io.projectreactor.logback (note: this one is under scrutiny to be dropped anyway)
  • reactor-netty:
    • io.projectreactor.netty (⚠️ older .ipc artifacts have generated name of reactor.netty so they shouldn't clash, but choosing this name would kind of enforce only being able to use a single 0.8+ netty module at at time. is that a problem @violetagg @smaldini ?)
  • reactor-rabbitmq:
    • io.projectreactor.rabbitmq
    • io.projectreactor.rabbitmq.samples
  • reactor-kafka:
    • io.projectreactor.kafka
    • io.projectreactor.kafka.samples
    • io.projectreactor.kafka.tools
  • reactor-tools:
    • io.projectreactor.tools
  • reactor-pool:
    • io.projectreactor.pool
  • BlockHound:
    • io.projectreactor.blockhound
    • io.projectreactor.blockhound.junit
  • reactor-scala-extensions: N/A
  • reactor-bom: N/A (this is purely a build artifact, not to be used as a JPMS module)

Please vote/comment @smaldini @violetagg @bsideup

  • 👍1
bsideup commented

io.projectreactor.reactor.core

Do we really need to duplicate "reactor" in it? io.projectreactor.reactor.core vs io.projectreactor.core

simonbasle commented

io.projectreactor.reactor.core

Do we really need to duplicate "reactor" in it? io.projectreactor.reactor.core vs io.projectreactor.core

Good point, there is only blockhound that doesn't have the reactor prefix, so we can remove it in module names I guess 👍. Will edit the proposal above.

simonbasle commented

Module Names Proposal 2

  • reactor-core: (reactor/reactor-core#1692)
    • reactor.core
    • reactor.test
  • reactor-addons: (reactor/reactor-addons#205)
    • reactor.adapter (exports packages reactor.adapter.akka and reactor.adapter.rxjava, NOT reactor.adapter)
    • reactor.extra
    • reactor.logback (note: dropped in 3.3.0.M1)
  • reactor-netty: (reactor/reactor-netty#731)
    • reactor.netty (⚠️ same as older .ipc artifacts that have generated name of reactor.netty)
  • reactor-rabbitmq: (reactor/reactor-rabbitmq@e10469ee)
    • reactor.rabbitmq
    • reactor.rabbitmq.samples
  • reactor-kafka: (reactor/reactor-kafka#94)
    • reactor.kafka
    • reactor.kafka.samples
    • reactor.kafka.tools
  • reactor-tools: TODO @bsideup, seems it's not even a complete build for now (no version)
    • reactor.tools
  • reactor-pool: (reactor/reactor-pool#44)
    • reactor.pool
  • BlockHound: (reactor/BlockHound#31)
    • reactor.blockhound
    • reactor.blockhound.junit
  • reactor-kotlin-extensions: N/A
  • reactor-scala-extensions: N/A
  • reactor-bom: N/A (this is purely a build artifact, not to be used as a JPMS module)

Please vote/comment @smaldini @violetagg @bsideup

  • 👍3
acogoluegnes commented

@simonbasle Reactor RabbitMQ already uses reactor.rabbitmq (the samples don't have any automatic module name though, I can add one). I did so to be consistent with the Spring Framework guidelines. I don't mind changing to be consistent with the Reactor libraries, but it'll have to wait until Reactor RabbitMQ 2.0.