GROUP_BASE_SKINS_R0

[todor]

GROUP_BASE_SKINS_R0

Ticket	Summary	Effort	Status
#346	BASE_SKINS_COMMONS_R0	1	closed
#350	BASE_SKINS_POINT_R0	1	closed
#353	BASE_SKINS_DEFAULT_SKIN_R0	1	closed
#356	BASE_SKINS_HACKERS_SKIN_R0	1	closed

[kyli]

2p. I think these things should be considered:

• the icon directory should not be .base.commons - they should be in the main.skins.alternative/src/resources/distrib/icons
• mode descriptive design for the new module - what classes will it contain, what will they contain. How will the new skin parts be added.
• documentation-related requirements are not satisfied.
• extensions/extension points mechanism for the skins are not described. For example, what should I do in order to write a new skin? Editing the base module is not acceptable in my opinion. These should not be hard-coded.
• We should ensure the default skin is full, not suppose it.
• design for skin configuring is missing (skin preferences, plugin/skin manager, etc). Isn't it a good idea for the skin manager to maintain a list with currently available skins, to have ability for switching skins, etc?

[nenko]

• the icon directory was not mentioned and now it is
• there's a text description about the class in the module and a package diagram
• brief explanation is added
• the default skin is the reference skin - it is full
• where to live a skin if not in a skin manager? To change a skin just set the skin you like in the currentSkin property.

[meddle]

The design is rewritten. Tests are missing. Because this task is a release implemented fix, and part of it's code is merged to the trunk, the implementation phase will be something like "fixing bugs, code quality and writing tests", so the design will pass now, but in the patch for the implementation phase have to have good tests and no bugs.

[pap]

An example to the errors in the class

[pap]

• The unit test has some problems
  • The constants for the skin values should be made real constants - final static fields with all capital letters
  • In the assertSame and similar methods you should first write the expected values and then the actual ones. Look at the method's tooltips when filling in arguments.
  • The @SuppressWarnings("static-access") is a bad thing you should instead use the ClassName.staticField convention.
  • The @SuppressWarnings("unchecked") should be only on the method and not on the class itself.
  • The @SuppressWarnings("synthetic-access") is not very nice
  • Correct the @author tag of the DemoModule class.
• The alternative icons are not where they are supposed to be (in the skins.alternative module)
• The AlternativeSkin menu item should be in the alternative skin module even if the menu is something temporal.
• The SkinManager class has unneeded commented out code and FAKE JavaDoc and its property is not initalized well. See the attached file for example.
• It is a good idea to add yourselves as authors of the Skin class (in the JavaDoc) because you made changes to its logic.

[nenko]

second try...

[nenko]

• Hope unit tests are fixed now
  • Now they are real CONSTANTS
  • assertSame will never be the same again
  • Bad things are now good
  • It's on the method now (over it)
  • You are right
  • The DemoModule is pap's, but now it's ours
• Fixed... I ran a sshd on my machine to copy them
• Moved
• Comments added... XXX is not a FAKE JavaDoc
• Isn't the author the man who wrote it, not the one who changed it...

[pap]

The implementation is acceptable but there are some drawbacks that were fixed by the integrators:

• There was still some missing JavaDoc.
• There were still several appearances of the word "hacker".

[deyan]

Batch update from file import.csv

[deyan]

Batch update from file query-obsoleted.csv