Ticket #1630 (closed unplanned_task: obsolete)

Opened 10 years ago

Last modified 10 years ago

GROUP_PERSISTENCE_R0

Reported by: boyan Owned by: mira
Priority: major Milestone: M05_PRE5
Component: BASE_PERSISTENCE Version: 2.0
Keywords: persistence Cc:
Category: BASE Effort: 0
Importance: 75 Ticket_group:
Estimated Number of Hours: Add Hours to Ticket:
Billable?: Total Hours:
Analysis_owners: boyan,gogov,deyan Design_owners: mira, deni
Imp._owners: Test_owners:
Analysis_reviewers: pav Changelog:
Design_reviewers: meddle, meddle Imp._reviewers: meddle
Test_reviewers: Analysis_score: 4
Design_score: 3 Imp._score: 3.5
Test_score: 0

Change History

comment:1 Changed 10 years ago by boyan

  • Effort changed from 12 to 0

comment:2 Changed 10 years ago by boyan

  • Description modified (diff)

comment:3 Changed 10 years ago by boyan

  • Status changed from new to s1a_analysis_started
  • Analysis_owners set to boyan

taken for analyzing

comment:4 Changed 10 years ago by boyan

  • Status changed from s1a_analysis_started to s1b_analysis_finished

done in 1h

comment:5 Changed 10 years ago by pav

  • Status changed from s1b_analysis_finished to s1c_analysis_ok
  • Analysis_score changed from 0 to 4

Full enough for me. 30m

comment:6 Changed 10 years ago by pav

  • Analysis_reviewers set to pav

comment:7 Changed 10 years ago by boyan

  • Status changed from s1c_analysis_ok to s2a_design_started

taken for designing

comment:8 Changed 10 years ago by pap

  • Description modified (diff)

fixing trac ticket query

comment:9 Changed 10 years ago by boyan

  • Status changed from s2a_design_started to s1c_analysis_ok

dropping the design

comment:10 Changed 10 years ago by deyan

  • Status changed from s1c_analysis_ok to new
  • Analysis_owners changed from boyan to boyan,gogov,deyan

Considered the analysis obsolete as it is more than a month old

comment:11 Changed 10 years ago by deyan

  • Owner changed from boyan to deyan
  • Status changed from new to s1a_analysis_started

added owners and started the analysis2

comment:12 Changed 10 years ago by gogov

  • Status changed from s1a_analysis_started to s1b_analysis_finished
  • Analysis_reviewers changed from pav to gogov,deyan

comment:13 Changed 10 years ago by gogov

  • Status changed from s1b_analysis_finished to s1c_analysis_ok

comment:14 Changed 10 years ago by deyan

  • Analysis_reviewers changed from gogov,deyan to pav

fixing

comment:15 Changed 10 years ago by deyan

design is started, this task is in progress

comment:16 Changed 10 years ago by mira

  • Owner changed from deyan to mira
  • Status changed from s1c_analysis_ok to s2a_design_started

comment:17 Changed 10 years ago by mira

  • Design_owners set to mira, deni

comment:18 Changed 10 years ago by mira

  • Status changed from s2a_design_started to s2b_design_finished

comment:19 Changed 10 years ago by meddle

  • Status changed from s2b_design_finished to s1c_analysis_ok
  • Design_score changed from 0 to 2
  • Design_reviewers set to meddle

I'll fail this design... I tried many things for something like 3 hours, but you have some major problems...

  • First of all the tests:
    • Read how to write tests! In the assertations there are rules for writing, expected and actual order and etc...
    • Spacing!
    • There have to be tests for all the persister implementations with provided revisions.
    • The provided revisions will be merged to the trunk after Design OK, so no warning and no errors, if you must, you can provide all your changesets up to now.
  • For the code quality
    • Spacing!
    • Accurate javadoc
    • And again spacing!
  • For the builds
    • I was fighting building your code to run with the True author/reader runner, you have to give me buildable code with maven.
    • Configure accurately your pom, the bundle activator is missing, the package export was no right, you have some ..*.persistence.storage package provided.

The code for the design is important and it have to be right, I wanted to pass that design and the design itself works, I gave too many time trying to fix the problems with the build. Please fix the problems, fix your tests, they are important, test all the new Resource creation methods, In one of the revisions with the new implementation I provided you such a test, write tests for the other methods.

2.5 (3h)

comment:20 Changed 10 years ago by mira

  • Status changed from s1c_analysis_ok to s2a_design_started

comment:21 Changed 10 years ago by mira

  • Status changed from s2a_design_started to s2b_design_finished

comment:22 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 meddle to meddle, meddle

My notes:

  • That is the design. When you add new methods to the Storage for example, write their accurate names...
  • When you write new logic, you describe it in the design, I don't see the new resource related methods from the ResourceSpace and the ResourceLocalCache described in the design, make sure at the implementation phase you describe them!
  • The file format : I think you should describe more of the attributes/tags in it, for example what is the meaning of the "index" attribute of the lists and why it is necessary can't we use the order of the tags instead, such things...
  • I think tests for this "refs" is needed, you use them in the higher-level tests, but you don't have their own test, besides that, you can test strange use cases like overriding their "get" method.
  • Other thing is that when you are saving the ImmSize for example you are using saving of Integers, so don't you thing the method for saving the size must be preceded by one for the integers in this case?
  • The use case in which you are saving text frame is important. I think in the BookPersistenceTest, you can add to the page of the book a text frame to clean this case.
  • I can see absolute paths in you tests like "C:
    blq.sjrb", fix them for the implementation phase!!!

But as a whole I can pass the design, the problems are mainly for the implementation. The persistence will run under the true author module, and the code quality is good, but I can not merge design related code at this state, and having in mind that the persistence task is a big one and the design and the implementation code are merged in one pile of code :D, I'll commit all of the persistence logic in the trunk after the implementation review passes.

3p (30m)

comment:23 Changed 10 years ago by mira

  • Status changed from s2c_design_ok to s3a_implementation_started

comment:24 Changed 10 years ago by mira

  • Status changed from s3a_implementation_started to s3b_implementation_finished

comment:25 Changed 10 years ago by meddle

  • Imp._score changed from 0 to 3.5
  • Imp._reviewers set to meddle

With the little fixes everything look right and works with the runners.

3.5p (100m)

comment:26 Changed 10 years ago by meddle

  • Status changed from s3b_implementation_finished to s3c_implementation_ok

comment:27 Changed 10 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.