Refactor Asset Wizard

[eselmeister]

The current implementation of the asset wizard is a nice idea:
#154

But it shows several flaws, which should be refactored:

• All the functionality is cluttered in one class "InstallAssetsHandler.java" -> that makes it hard to test the code
• Enums should be created separately instead of using inner enums -> AssetType
• Classes should be created separately instead of using inner classes -> AssetItem
• Direct member access should be avoided: "delete.file.getName()" -> the separation of enums and classes unveils this
• Enum -> items shall use upper case letters
• File Dialog -> All Asset selection text looks weird
• The wizard only shows basic functionality

Whlie refactoring, the following issues shall be addressed too:

• "Remove All" action -> remove all asset(s)
• "Import Zipped Assets" action -> to allow to add assets, bundles in a zip file

[laeubi]

@eselmeister You should check the header of extracted classes they are obviously not "initial API and implementation" by lablicate/Philip Wenig but mostly copied from the original implementation and the adjusted....

[eselmeister]

@laeubi Unfortunately, the code was so murky, that most parts required a re-write from scratch.

[laeubi]

I'll check with Eclipse staff then whats the process here, its obvious that you have not written everything from scratch as names, methods and parts of the code are way to similar as to be "rewritten from scratch"...

[eselmeister]

@laeubi It took me half a day to fix your code. For review/comparison purposes, I have tried to refactor the code as closely aligned to existing code as possible. Test cases, classes and the enum have been created from scratch. AssetItem ... you can simply create getter/setters via the Eclipse tooling. The AssetInstallPage needed a re-write too, as it was placed inside the handler "InstallAssetsHandler" and the functionality wasn't well separated ... calling member directly, and so on.

[waynebeaton]

The short version is that when you split a file during a refactoring exercise, new files automatically inherit the header of the original. If even a fragment of a single line of the original content survives the refactoring (whether generated, rote, or otherwise), then the original copyright statement and author information should be retained.

As others add their content, the copyright statement you can either add the new copyright holders to the existing copyright statement or append "and others" (especially when the list of copyright holders gets long). There's more help (and an alternative header option) in the handbook.

Note that the list of contributors is considered optional by the Eclipse Foundation.

Note also that the escalation path starts with the PMC to request their help in resolving any dispute.

[eselmeister]

@waynebeaton That's a good point. I'll take care to have a look at the headers when refactoring the code.

@laeubi Please take care of your code quality, do testing and please respond to questions raised in the issues instead of ignoring them, e.g.:

#154
#408
#405

[laeubi]

All mentioned issues are resolved form my point of view, if you still feel uncomfortable with them feel free to improve according to your own 'standards'.

[eselmeister]

@laeubi Are #408 and #405 solved?

[laeubi]

408: See #408 (comment) for how system-methods (either manually installed or installed via assets manager) are supposed to work -> works as designed, only issue was that .bak files are detected as methods, that was resolved in 42f9b55 if anything is still unclear feel free to ask.

405: was closed/resolved until you reopened it because you feel the naming is wrong, if your feeling that it's better renamed don't hesitate to do so

[eselmeister]

@laeubi

#408: There is still no comment, why you have just closed the related issue without providing any contribution or explanation, why the issue has been set to closed.

#405: The bundle contains SWT (UI) dependency, hence please rename it to "org.eclipse.chemclipse.cli.ui". You are a ChemClipse project lead, hence you should be familiar with the naming conventions.

[eselmeister]

Let's close this chat here and proceed in #408 and #405.

[laeubi]

There are no "related issues" that the Chromatogram Menu does not behaves nicely has nothing to do with the system methods, nor with the assets manager and even not with chemclipse itself as the issue you are refering to are in swt-chart.

[eselmeister]

@waynebeaton
@laeubi I disagree, that those things are not related. Anyway, even if #408 is solved in your opinion, it still doesn't answer the question why you have closed eclipse/swtchart#84 without any further comment.