Clean Code for team/bundles/org.eclipse.compare.win32#1928
Conversation
|
This pull request changes some projects for the first time in this development cycle. Warning 🚧 This PR cannot be modified by maintainers because edits are disabled or it is created from an organization repository. To obtain the required changes apply the git patch manually as an additional commit. Git patchFurther information are available in Common Build Issues - Missing version increments. |
8b54507 to
bd441ba
Compare
HeikoKlare
left a comment
There was a problem hiding this comment.
It seems like the Tycho cleanup mojo may not be applicable to bundles that do not fit to the current platform and that depend on some other code/bundle that does not fit to the current platform. In this case, a win32-specific bundle is cleaned up on Linux, which erroneously removes a bunch of imports, probably because it compiles against the Linux fragment of SWT which does obviously not contain win32-specific classes like Ole classes. So in my understanding, the issue is not related to a bundle just being platform-specific, but to a bundle being platform-specific and depending on some other platform-specific code/bundle.
When I execute the cleanup via Tycho for the org.eclipse.compare.win32 bundle on a Windows system, it produces a proper result.
@laeubi does the assessment make sense to you?
I am not sure what would be the right way do deal with it. Making the cleanup mojo only execute if the platform filter of a bundle fits to the current platform would be too restrictive (if always done in a non-configurable way), as a platform-specific bundle itself may not be problematic. But since the we can also properly compile this win32-specific bundle on Linux, it may be possible to also enhance the cleanup mojo to perform the cleanup against dependencies according to the bundle's platform filter?
0174ee6 to
c7e9145
Compare
This is absolutely possible!
I need to take a look, in general is should be possible to some extend as we basically setup a java project and then call the cleanup actions, even if the native stuff will never run, JDT should be happy by just see the java class itself... |
9c6ff37 to
7beabba
Compare
ab60b5c to
ea8727c
Compare
|
@HeikoKlare this should fix the issue seen here: |
de4eca7 to
5460e52
Compare
f964484 to
b23b00f
Compare
|
All is good now. Thanks for the enhancements to Tycho! |
|
Thank you for the Tycho improvement that made this work properly, @laeubi! |
The following cleanups were applied: