Ticket #448 (closed planned_task: obsolete)

Opened 11 years ago

Last modified 10 years ago

APP_RESOURCE_LIST_IMPORT_EXPORT_R1

Reported by: Astea Owned by: george
Priority: 3 Milestone: M07_ALPHA2
Component: APP_RESOURCE_MANAGEMENT Version: 2.0
Keywords: Cc: george
Category: MAIN Effort: 1
Importance: 0 Ticket_group:
Estimated Number of Hours: 0 Add Hours to Ticket: 0
Billable?: yes Total Hours: 0
Analysis_owners: deyan, nenko Design_owners: george
Imp._owners: george Test_owners:
Analysis_reviewers: dido Changelog:
Design_reviewers: meddle, pap, meddle Imp._reviewers: meddle, todor
Test_reviewers: Analysis_score: 4
Design_score: 4 Imp._score: 3
Test_score: 0

Description (last modified by george) (diff)

wiki page: APP_RESOURCE_LIST_IMPORT_EXPORT_R1 - effort: 9d

Change History

comment:1 Changed 10 years ago by deyan

  • Status changed from new to s1a_analysis_started
  • Analysis_score set to 0
  • Test_score set to 0
  • Design_score set to 0
  • Owner changed from Astea to deyan
  • Imp._score set to 0
  • Analysis_owners set to deyan, nenko

comment:2 Changed 10 years ago by nenko

  • Status changed from s1a_analysis_started to s1b_analysis_finished

comment:3 Changed 10 years ago by dido

  • Status changed from s1b_analysis_finished to s1c_analysis_ok
  • Analysis_reviewers set to dido
  • Analysis_score changed from 0 to 4
  • Think its a good idea to add button inside resource palette that allows importing of all kind of resources. But instead of inserting them on the stage, it will be better to import them inside the book, without creating instances on the current page.
  • I expect this will work in the same manner for all insert/import dialogs.
  • Please confirm what will be achieved on this topics in this revision before continue with design. If there is a need change the analysis, otherwise add comment on the wiki page.
  • Everything else its more than fine.

Analysis reviewed 4p (30m)

comment:4 Changed 10 years ago by nenko

  • Owner changed from deyan to nenko
  • Status changed from s1c_analysis_ok to s2a_design_started
  • Importing resources only will be added in later revision of the task.
  • Yes, inserting will work in the same manner

comment:5 Changed 10 years ago by dido

  • Lovely :)

comment:6 Changed 10 years ago by george

  • Design_owners set to george
  • Status changed from s2a_design_started to s2b_design_finished
  • Description modified (diff)
  • Total Hours set to 0
  • Add Hours to Ticket set to 0
  • Billable? set
  • Estimated Number of Hours set to 0

comment:7 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, pap
  • Design related things from pap:
    • FileManager is not very nice name for me. It is too general.
    • Are you sure that the warnings about raw types should be fixed by putting a " @SuppressWarnings("unchecked") "
    • Actually you don't need to set the source class to AppMenuItem. Leaving it to a concrete subclass should also work and will reduce computation time
    • Having these filters defined in FileDialogInput is bad. It breaks the idea of extensibility for me.
    • Using "FileDialogInput.ANNOTATION_FILTER" to check for zip file is quite incorrect.
    • Wouldn't it be better if you make an extension point for file openers. Its interface could have something like {getFileFilter(), getKind()}
  • Code related notes from meddle:
    • Your code have no spacing, bad identation and strange things in it... see the code in GoodCodeExamples and fix yours.
    • Change
      	if(files[i].toString().charAt(files[i].toString().length() - 1) == '~'){
      		//opened text files generate a temporary file, ending with ~
      		continue;
      	}
      	unsupportedFiles += files[i].toString() + "\n";
      

to

	if (files[i].toString().charAt(files[i].toString().length() - 1) != '~') {
		unsupportedFiles += files[i].toString() + "\n";
	}
	
  • Don't swallow exceptions like this:
    	try {
    		zf = new ZipFile(zipFile);
    	} catch (Exception e) {
    		return null;
    	}
    
  • Some editor's temporary files don't end with '~'... But that is not important :)
  • hyphenate maximum at the 100th character of the line!!!
  • In the tests:
    • You have no test for your new FileManager (rename it!) write good one, your new logic is there after all.
    • assertTrue(pwa.elementViews().get().size() == 7); change this line to assertTrue(7, pwa.elementViews().get().size());, that way they are better for readers.
  • Overall:
    • We don't like that the FileManager dispatches ADD_NEW_FRAME. Imagine you import a group of resources or one resource only to the resource palette with dran and drop for example, we wouldn't be able to use your manager... A better aproach is to dispatch something like OPEN_FILE event and the logics for adding frames to listen to it and dispatch events... I think that is important.
    • Write good SVN comments when commiting like #TICKET_NUMBER TICKET_NAME {What did you change}

comment:8 Changed 10 years ago by george

  • Owner changed from nenko to george
  • Status changed from s1c_analysis_ok to s2a_design_started

comment:9 Changed 10 years ago by george

  • Description modified (diff)

comment:10 Changed 10 years ago by george

  • Status changed from s2a_design_started to s2b_design_finished

comment:11 Changed 10 years ago by meddle

  • Status changed from s2b_design_finished to s2c_design_ok
  • Design_score changed from 2 to 4
  • Design_reviewers changed from meddle, pap to meddle, pap, meddle

OK, it seems you took seriously our comments, so this design will pass now. But be aware of the following:

  • Your code MUST run with the true runners, we will test it.
  • Your code MUST not break anything that is working now (be careful with the book saving again)
  • Don't write "implementation" in the design section, it's the wrong way at design to have implementation, which is your case... Write "design related code".
  • For the implementation review provide all the needed revisions, because if your code is merged with compilation errors or warnings, it will fail...

4p (30m)

comment:12 Changed 10 years ago by meddle

  • Cc george added

comment:13 Changed 10 years ago by george

  • Status changed from s2c_design_ok to s3a_implementation_started

comment:14 Changed 10 years ago by george

  • Imp._owners set to george

comment:15 Changed 10 years ago by george

  • Status changed from s3a_implementation_started to s3b_implementation_finished

comment:16 Changed 10 years ago by meddle

  • Status changed from s3b_implementation_finished to s3c_implementation_ok
  • Imp._score changed from 0 to 3
  • Imp._reviewers set to meddle, todor

I pass it, but I didn't like some things in the code like hyphenation on comma and the fake javadod, especially the fake JavaDoc...

3p (80m)

comment:17 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.