Remove package export from SVG fragment #2602
Conversation
HeikoKlare
force-pushed
the
jsvg-remove-export
branch
from
0281193 to
cd9e134
Compare
| com.github.weisj.jsvg.parser;version="[2.0.0,3.0.0)", | ||
| com.github.weisj.jsvg.view;version="[2.0.0,3.0.0)" | ||
| Export-Package: org.eclipse.swt.svg | ||
| Export-Package: org.eclipse.swt.svg;x-friends:="org.eclipse.swt.tests" |
There was a problem hiding this comment.
I suggest to not add the tests as a friend but the mark the package as x-internal entirely and suppress the warning in the test instead.
Having the tests mentioned in the host bundle is IMO not clean and has the potential to be left-over in case the friend is not necessary anymore.
There was a problem hiding this comment.
That's true. I actually though about entirely moving the tests to the fragment, as that's where they belong (since they do not test API and since we cannot have a test fragment for a fragment). But it would require to add the same additional configuration we have added for tests inside the win32 fragment and I was not sure whether it's worth for just this fragment, so it's probably fine to suppress the restriction warnings instead.
There was a problem hiding this comment.
That sounds good in general, but I agree that it's benefit is questionable.
HeikoKlare
force-pushed
the
jsvg-remove-export
branch
2 times, most recently
from
5796731 to
4f0e75b
Compare
| com.github.weisj.jsvg.parser;version="[2.0.0,3.0.0)", | ||
| com.github.weisj.jsvg.view;version="[2.0.0,3.0.0)" | ||
| Export-Package: org.eclipse.swt.svg | ||
| Export-Package: org.eclipse.swt.svg;x-internal:=true |
There was a problem hiding this comment.
I think the commit message is confusing... this does not remove the export it only marks it as internal ...
There was a problem hiding this comment.
Thanks! I didn't properly update the message when replacing the previous x-friends. Do we need the internal flag at all or couldn't we just completely remove the export? What's the benefit of keeping it like this?
There was a problem hiding this comment.
The export is only needed if you import it somewhere of course... not sure where is is really used, if its only for test thex look to me that they can be plain surefire tests without OSGI and then package exports should not matter at all (you might want jar.extra classpath instead to make PDE IDE happy)
There was a problem hiding this comment.
The only usage is in org.eclipse.swt.test and there we don't use a package import, so it should not matter? At least I do not see any complain in my IDE when simply removing the export here.
And yes, those tests could be plain surefire tests and could also be placed inside that fragment, as discussed in the comments above: #2602 (comment)
There was a problem hiding this comment.
Okay, the build properly complains, only my IDE did not. I changed to x-internal again and adapted the commit message.
HeikoKlare
force-pushed
the
jsvg-remove-export
branch
2 times, most recently
from
916a304 to
39d445d
Compare
This mark the package export for org.eclipse.swt.svg from the SVG fragment as internal because it's only required by tests.
HeikoKlare
force-pushed
the
jsvg-remove-export
branch
from
39d445d to
2fa8d0a
Compare
3 participants
This removes the unnecessary package export for org.eclipse.swt.svg from the SVG fragment. It limits usage to the test bundle in which tests of the JSVG rasterizer class reference that class.
I've also added a PR for Platform UI with API filters for the bundles reexporting SWT, which has to be merged together with this one (or right after):