Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#51 Add jpms module-info using Multi-Release jar #59

Merged
merged 5 commits into from
Oct 30, 2020

Conversation

rbygrave
Copy link
Contributor

No description provided.

inject/README.md Outdated Show resolved Hide resolved
inject/src/main/java9/module-info.java Show resolved Hide resolved
inject/README.md Outdated Show resolved Hide resolved
@norrisjeremy
Copy link
Contributor

Have you given any thought to migrating to new jakarta.inject / jakarta.annotation-api? It might be good time to do the migration while you are introducing JPMS.

@rbygrave
Copy link
Contributor Author

Have you given any thought to migrating to new jakarta.inject / jakarta.annotation-api?

I did a little bit. jakarta.inject uses Automatic-Module-Name in the manifest.

It might be good time to do the migration while you are introducing JPMS.

Well I'm still learning but where I'm at at the moment is that I think it is ok for some modules to be "unnamed" ones. My working theory is that unnamed modules are ok in the case that:

  • It has no dependencies
  • If it had a module-info it would just contain ... export * / export all packages.

As I see it right now, both javax.inject and java.annotation-api match this description (and my example jpms app is fine with them as they are). So right now if I was doing a full jpms app those modules are actually fine to use as is. I currently don't see anything to gain from say forking and making a jpms version of them (some people have done this) or moving to say jakarta.inject (even if they had a module-info which they don't).

Hmmm.

@norrisjeremy
Copy link
Contributor

Well I'm still learning but where I'm at at the moment is that I think it is ok for some modules to be "unnamed" ones. My working theory is that unnamed modules are ok in the case that:

  • It has no dependencies
  • If it had a module-info it would just contain ... export * / export all packages.

As I see it right now, both javax.inject and java.annotation-api match this description (and my example jpms app is fine with them as they are). So right now if I was doing a full jpms app those modules are actually fine to use as is. I currently don't see anything to gain from say forking and making a jpms version of them (some people have done this) or moving to say jakarta.inject (even if they had a module-info which they don't).

Hmmm.

Part of my thought process for suggesting a migration to the new 2.x jakarta.inject + jakarta.annotation-api versions are two-fold:

  1. I believe the module names for the old javax versions are technically reserved by the JDK itself (javax.inject / java.annotation). While I would think it's unlikely Oracle would decide to add modules with colliding names into the JDK itself, it's possible they could. And then I think hilarity could ensue, since there would be conflicting module names.

  2. With adding in the JPMS support, it might be best to rip the bandaid off entirely all at once, and shift everything to using the new jakarta.inject + jakarta.annotation package names all at the same time. This would be admittedly highly disruptive to users of this library, since they would need to adjust all their classes to use the jakarta.inject + jakarta.annotation annotations instead of javax.inject + javax.annotation.

Anyway, that was just part of my thinking at the moment (while reviewing our internal projects and determining how best to migrate them from Guice to this library).

@rbygrave
Copy link
Contributor Author

it might be best to rip the bandaid off entirely all at once, and shift everything to using the new jakarta.inject + jakarta.annotation package names all at the same time.

Maybe, although with this change it still supports java 8 and classpath. This change doesn't demand anything from it's users per say. It's ready for java modules but the majority are still going to be classpath for a while I'm suspecting.

This would be admittedly highly disruptive to users of this library,

Well it would.

What I have in my mind is that there might literally be 2 changes to go in with:
A) this change
B) maybe rename core package to spi. (pondering this)

Once those are in this library could be very stable and literally only need bug fixes. So my thinking at the moment is to get to there ... be nice and stable and boring ... wait for demand to go to jakarta.inject.

Ideally the only change after A) and B) is ... user demand to migrate to jakarta.inject (from javax.inject).

@norrisjeremy
Copy link
Contributor

Well it would.

What I have in my mind is that there might literally be 2 changes to go in with:
A) this change
B) maybe rename core package to spi. (pondering this)

Once those are in this library could be very stable and literally only need bug fixes. So my thinking at the moment is to get to there ... be nice and stable and boring ... wait for demand to go to jakarta.inject.

Ideally the only change after A) and B) is ... user demand to migrate to jakarta.inject (from javax.inject).

Yep, that probably makes the most sense. I tend to think too big sometimes :).

@rbygrave
Copy link
Contributor Author

rbygrave commented Oct 29, 2020

Although maybe we could/should have both by maintaining 2 branches - a branch for javax.inject based dependencies and a branch for jakarta.inject dependencies.

My situation sounds similar with existing Guice, HK2 (Jersey) and Spring - and the migration is not always full replacement [of Guice] but where there is co-existence moving towards replacement over time. Now with co-existence I think it would have been nicer/easier to use jakarta.inject so that there was clear distinction.

Plan

  • Release what's in master as 1.4
  • Merge in the jpms change, refactor rename core to spi - release as 2.0
  • Create a long lived branch called "jakarta" and in that branch move to jakarta dependencies, release off that branch as 3.0

Developers can then choose version 2.0 (javax.inject) or 3.0 (jakarta.inject)
Bug fixes going forward need to apply to both branches and release 2.x and 3.x when that occurs.

@norrisjeremy
Copy link
Contributor

  • Release what's in master as 1.4
  • Merge in the jpms change, refactor rename core to spi - release as 2.0
  • Create a long lived branch called "jakarta" and in that branch move to jakarta dependencies, release off that branch as 3.0

Developers can then choose version 2.0 (javax.inject) or 3.0 (jakarta.inject)
Bug fixes going forward need to apply to both branches and release 2.x and 3.x when that occurs.

That sounds like it could be workable plan. It probably wouldn't be too difficult to keep any changes in sync between 2.x branch & 3.x branch, since I think the main difference would just be a search/replace of javax.inject => jakarta.inject & javax.annotation => jakarta.annotation.

@rbygrave rbygrave added this to the 2.0 milestone Oct 30, 2020
@rbygrave rbygrave merged commit 9a0e249 into master Oct 30, 2020
@rbygrave rbygrave deleted the feature/53-moduleInfo branch November 16, 2020 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants