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

Create Payara POM #4062

Closed
wants to merge 8 commits into from
Closed

Create Payara POM #4062

wants to merge 8 commits into from

Conversation

pdudits
Copy link
Contributor

@pdudits pdudits commented Jun 26, 2019

The BOM contains all third-party dependencies of Payara codebase, that get copied into modules directory, as well as public artifacts that are published.

This should prevent ecosystem projects as well as users not import incompatible third-party or transitive dependencies.

This is what changed:

  • All version properties are in root pom (this one needs to get released)
  • All dependency management moved to payara-bom (this would also be released)
  • nucleus parent is not the root pom, java.net parent was dropped.
  • nucleus imports bom, having all of the modules inherit it.

@pdudits
Copy link
Contributor Author

pdudits commented Jun 28, 2019

jenkins test please

@jbee
Copy link
Contributor

jbee commented Jun 28, 2019

java.net parent was dropped.

🎉

Copy link
Member

@Pandrex247 Pandrex247 left a comment

Choose a reason for hiding this comment

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

Couple of questions

<jcip-annotations.version>1.0-1</jcip-annotations.version>
<!-- Yubico client validation version property -->
<yubico-validation-client2.version>3.0.2.payara-p1</yubico-validation-client2.version>

<dockerfile-maven-plugin.version>1.4.10</dockerfile-maven-plugin.version>
Copy link
Member

Choose a reason for hiding this comment

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

What's the reasoning behind some properties being left in this POM?
E.g. this one, and the javaee.major_version property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it's because they didn't affect any content in modules folder. Even though it's bit surprising for javaee.major_version, isn't it?

Copy link
Member

Choose a reason for hiding this comment

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

I imagine you can get away with that one as nucleus is supposed to be pure application server, with appserver being the additional stuff to turn it into a Java / Jarkarta EE application server.

<groupId>org.glassfish.main</groupId>
<artifactId>glassfish-main-aggregator</artifactId>
<groupId>fish.payara</groupId>
<artifactId>payara-main-aggregator</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to take this chance to drop the main part?
That's a GlassFish holdover I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good idea. But now I'm also thinking about groupId fish.payara.server again, or actually fish.payara.main

Copy link
Member

Choose a reason for hiding this comment

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

I prefer server over main.
main implies that there's something that isn't.

Alternatively you can simply go with fish.payara as the groupId for the top-level pom, then fish.payara.nucleus and fish.payara.appserver in the respective folders. That's just me spit-balling though

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe its a good chance to move away from nucleus? Its a word that only makes sense if you know what is meant in the context in which case any word would work. Instead we could use words that communicate something even to people who do not know the context.

@pdudits
Copy link
Contributor Author

pdudits commented Jul 31, 2019

This is now conflicting with jakarta dependencies being brought in, and it is also not a good thing to add in last sprint of a release. Will redo that for 194 next month

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants