Ticket #1672 (closed unplanned_task: obsolete)

Opened 10 years ago

Last modified 9 years ago

GROUP_BASE_SKINS_R0

Reported by: todor Owned by: nenko
Priority: major Milestone: M07_ALPHA2
Component: BASE_SKINS Version: 2.0
Keywords: Cc:
Category: BASE Effort: 0
Importance: Ticket_group:
Estimated Number of Hours: Add Hours to Ticket:
Billable?: Total Hours:
Analysis_owners: todor Design_owners: nenko, peko,todor
Imp._owners: nenko, peko Test_owners:
Analysis_reviewers: nenko Changelog:
Design_reviewers: kyli, meddle Imp._reviewers: meddle,pap
Test_reviewers: Analysis_score: 3.5
Design_score: 3 Imp._score: 3
Test_score: 0

Attachments

SkinManager.java (873 bytes) - added by pap 10 years ago.
An example to the errors in the class

Change History

comment:1 Changed 10 years ago by todor

  • Description modified (diff)

comment:2 Changed 10 years ago by nenko

  • Owner set to nenko
  • Status changed from new to s1a_analysis_started

comment:3 Changed 10 years ago by nenko

  • Status changed from s1a_analysis_started to s1b_analysis_finished

comment:4 Changed 10 years ago by nenko

  • Analysis_owners set to todor

comment:5 Changed 10 years ago by nenko

  • Status changed from s1b_analysis_finished to s1c_analysis_ok
  • Analysis_reviewers set to nenko
  • Analysis_score changed from 0 to 3.5

comment:6 Changed 10 years ago by nenko

  • Design_owners set to nenko, peko
  • Status changed from s1c_analysis_ok to s2a_design_started

comment:7 Changed 10 years ago by nenko

  • Status changed from s2a_design_started to s2b_design_finished

comment:8 Changed 10 years ago by todor

  • Design_owners changed from nenko, peko to nenko, peko,todor

comment:9 Changed 10 years ago by kyli

  • Status changed from s2b_design_finished to s1c_analysis_ok
  • Design_score changed from 0 to 2
  • Design_reviewers set to 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?

comment:10 Changed 10 years ago by nenko

  • Status changed from s1c_analysis_ok to s2a_design_started

comment:11 Changed 10 years ago by nenko

  • Status changed from s2a_design_started to s2b_design_finished
  • 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.

comment:12 Changed 10 years ago by meddle

  • Status changed from s2b_design_finished to s2c_design_ok
  • Design_score changed from 2 to 3
  • Design_reviewers changed from kyli to kyli, 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.

comment:13 Changed 10 years ago by nenko

  • Status changed from s2c_design_ok to s3a_implementation_started
  • Imp._owners set to nenko. peko

comment:14 Changed 10 years ago by nenko

  • Status changed from s3a_implementation_started to s3b_implementation_finished

Changed 10 years ago by pap

An example to the errors in the class

comment:15 Changed 10 years ago by pap

  • Cc peko, nenko added
  • Status changed from s3b_implementation_finished to s2c_design_ok
  • Imp._score changed from 0 to 1
  • Imp._reviewers set to meddle
  • 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.

comment:16 Changed 10 years ago by nenko

  • Status changed from s2c_design_ok to s3a_implementation_started

second try...

comment:17 Changed 10 years ago by nenko

  • Status changed from s3a_implementation_started to s3b_implementation_finished
  • 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...

comment:18 Changed 10 years ago by pap

  • Status changed from s3b_implementation_finished to s3c_implementation_ok
  • Imp._score changed from 1 to 3
  • Imp._reviewers changed from meddle to meddle,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".

comment:19 Changed 10 years ago by meddle

  • Imp._owners changed from nenko. peko to nenko, peko

comment:20 Changed 10 years ago by deyan

  • Cc peko, nenko removed

Batch update from file import.csv

comment:21 Changed 9 years ago by deyan

  • Status changed from s3c_implementation_ok to closed
  • Resolution set to obsolete

Batch update from file query-obsoleted.csv

Note: See TracTickets for help on using tickets.