Adding SWT + JFace when creating a plugin that "makes contribution to the UI" #1306 #1472
Conversation
Test Results 285 files ±0 285 suites ±0 52m 21s ⏱️ - 1m 17s For more details on these errors, see this check. Results for commit be4dc0b. ± Comparison against base commit ef694eb. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Hi @cervantes-yadira, thank you for this PR. It already looks quite good. Nice work.
I just have a comment in the code to further refine this.
If you apply the changes, please amend your commit locally and force push the amended commit to the branch of this PR so that we get a nice and clean history.
| } | ||
| // Else if a plug-in project is being created... | ||
| else if (fData instanceof IPluginFieldData) { | ||
| // And the plug-in project makes contributions to the UI... | ||
| if (((IPluginFieldData) fData).isUIPlugin()) { | ||
| // Add SWT and JFace as dependencies | ||
| result.add(new PluginReference("org.eclipse.swt")); //$NON-NLS-1$ | ||
| result.add(new PluginReference("org.eclipse.jface")); //$NON-NLS-1$ | ||
| } | ||
| } |
There was a problem hiding this comment.
You can use Pattern matching for instanceof here and then combine the two if statements nicely into one.
Furthermore the comments can be reduced here. It's not necessary to write down every action in a corresponding comment. Comments should only be used to clarify the logic where it is not obvious from the code (ideally the code is readable and understandable in itself).
And last but not least the logic should be enhanced to to not add the dependencies if a user has selected a template.
If a template is selected that template already defines the necessary dependencies and nothing should be change here.
Just as described in #1306 (comment).
| } | |
| // Else if a plug-in project is being created... | |
| else if (fData instanceof IPluginFieldData) { | |
| // And the plug-in project makes contributions to the UI... | |
| if (((IPluginFieldData) fData).isUIPlugin()) { | |
| // Add SWT and JFace as dependencies | |
| result.add(new PluginReference("org.eclipse.swt")); //$NON-NLS-1$ | |
| result.add(new PluginReference("org.eclipse.jface")); //$NON-NLS-1$ | |
| } | |
| } | |
| } else if (fContentWizard == null && fData instanceof IPluginFieldData pluginData && pluginData.isUIPlugin()) { | |
| // a plug-in project is being created and makes contributions to UI | |
| result.add(new PluginReference("org.eclipse.swt")); //$NON-NLS-1$ | |
| result.add(new PluginReference("org.eclipse.jface")); //$NON-NLS-1$ | |
| } |
There was a problem hiding this comment.
Thanks for the suggestions! I added the additional check for the use of templates and used pattern matching as you suggested. I have appended the commit and pushed it to this branch.
cervantes-yadira
force-pushed
the
master
branch
from
874f83e
to
9399021
Compare
Dependencies aren't added for plug-ins that generate an activator file or use a template.
cervantes-yadira
force-pushed
the
master
branch
from
9399021
to
be4dc0b
Compare
According to the original issue, using JFace and SWT is almost necessary for plugins that contribute to the UI. Since these dependencies can be easily removed and are already dependencies of Eclipse, we decided to add them by default.
Fixes #1306