-
Notifications
You must be signed in to change notification settings - Fork 822
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
Update eclipselink from 2.7.9 to 2.7.10 #3646
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general this looks good. There is one change, that I don't understand, please see inline.
@@ -26,29 +26,126 @@ | |||
<code-name-base>org.netbeans.modules.j2ee.eclipselink</code-name-base> | |||
<module-dependencies/> | |||
<public-packages> | |||
<subpackages>org.eclipse</subpackages> | |||
<subpackages>org.eclipse.persistence</subpackages> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for this change? Are there internal packages, that shall be hidden? If only a single package should be exposed, I would expect package
, subpackages
matches all packages where the given name is a prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was under the impression that I should only include subpackages
that have java classes inside and exclude internal packages like org.eclipse.persistence.internal
. What would be the proper way of configure <public-packages>
and <subpackages>
, include all packages excluding internal packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to expose only the package and not the package where it is a prefix of, package
is the right element. Your assessment is right, internal should not be exposed.
The sigtest fails from my interpretation because classes are missing:
https://app.travis-ci.com/github/apache/netbeans/jobs/560459188#L12806-L12808
This smells like these:
https://search.maven.org/artifact/jakarta.resource/jakarta.resource-api/1.7.4/jar
https://search.maven.org/artifact/jakarta.transaction/jakarta.transaction-api/1.3.3/jar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I restore the entry <subpackages>org.eclipse</subpackages>
, deleted the added entries, regenerate sigtest and everything worked fine. I suppose that if another module need a class from this packages it would fail the build process.
sigtests are failing on this PR even after restart. |
84382f0
to
6ce87ba
Compare
-This is mainly a maintenance release and contains bug fixes -Use jakarta.persistence instead of javax.persistence (still using javax.* in packages) -Regenerate signatures
6ce87ba
to
539988c
Compare
tests are green, PR is approved / no outstanding change requests as far as I see -> merging. |
Library Notes:
NetBeans Testing:
websvc.restlib
,j2ee.eclipselink
,j2ee.eclipselinkmodelgen
,j2ee.jpa.refactoring
,j2ee.metadata.model.support
,j2ee.persistence
andj2ee.persistenceapi
EclipseLink Web Page
EclipseLink Release Notes