Ticket #1764 (closed planned_task: obsolete)

Opened 10 years ago

Last modified 10 years ago

GROUP_SCRIPTING_R0

Reported by: mitex Owned by: deni
Priority: major Milestone: M10_BETA2
Component: SCRIPTING Version: 2.0
Keywords: script, javascript Cc:
Category: EXTRA Effort:
Importance: Ticket_group:
Estimated Number of Hours: Add Hours to Ticket:
Billable?: Total Hours:
Analysis_owners: deni, dido, mitex Design_owners: mitex, deni
Imp._owners: deni, mitex Test_owners:
Analysis_reviewers: Changelog:
Design_reviewers: meddle, pap Imp._reviewers: meddle, pap
Test_reviewers: Analysis_score: 3
Design_score: 3.5 Imp._score: 3.5
Test_score: 0

Change History

comment:1 Changed 10 years ago by mitex

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

comment:2 follow-up: ↓ 3 Changed 10 years ago by mitex

  • Status changed from s1a_analysis_started to s1b_analysis_finished

analysis finished (1h)

comment:3 in reply to: ↑ 2 Changed 10 years ago by mitex

Replying to mitex:
analysis finished (2h)

comment:4 Changed 10 years ago by dido

The editing of scripts via Sophie should be included in this revision.

comment:5 Changed 10 years ago by deyan

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

These two subrequirements overlap each other

  • Add 'Insert Script File' button inside 'Insert' menu
  • Create 'Insert Script' button inside 'Insert' menu that creates new resource.

The right one is Insert -> Script, because the word "file" is not met in the insert menu

  • The choise of a scripting language is just mentioned in the implementation idea, I think this is very important for the overview of the task.
  • The button that appears only when a script is selected, should be rethought - currently, the policy is to add inactive buttons insead of invisible.

comment:6 Changed 10 years ago by mitex

  • Design_owners set to deni, mitex
  • Status changed from s1c_analysis_ok to s2a_design_started
  • Imp._owners set to deni, mitex

comment:7 Changed 10 years ago by mitex

  • Design_owners changed from deni, mitex to mitex, deni
  • Status changed from s2a_design_started to s2b_design_finished

comment:8 Changed 10 years ago by meddle

  • Status changed from s2b_design_finished to s2c_design_ok
  • Design_score changed from 0 to 3.5
  • Design_reviewers set to meddle, pap

Code related notes:

  • Remove storage.debugPrint(0); from your code for review.
  • Use AutoVisualProviders.
  • Hmmm that's not good: ScriptResource resource = model().get().get(ScriptResource.class);, use the current book to get the script resource like ScriptResource resource = model().get().get(ScriptResource.class, CurrentBookUtil.getCurrentBook());
  • Umm you use the create method of the ResourceProperty in strange way:
            @Override
            protected Object create() {
                    JPanel panel = swingComponent().get();
                   
                    JScrollPane scrollPane = new JScrollPane(textArea().get());
                    panel.add(scrollPane);
                   
                    JButton runButton = new LogicR3Button("Run", new EventR3(
                                ScriptDocumentWindow.this, null, null, null,
                                EventIds.RUN_SCRIPT));
                    panel.add(runButton);
                   
                    return new Object();
            }
    
    // RIGHT:
            @Override
            protected JPanel create() {
                    JPanel panel = swingComponent().get();
                   
                    JScrollPane scrollPane = new JScrollPane(textArea().get());
                    panel.add(scrollPane);
                   
                    JButton runButton = new LogicR3Button("Run", new EventR3(
                                ScriptDocumentWindow.this, null, null, null,
                                EventIds.RUN_SCRIPT));
                    panel.add(runButton);
                   
                    return panel;
            }
    
  • Remove the System.out.printlns form the code for review.
  • You added a new module, for the implementation review make sure that everything is OK. See Integration/CheckList.

Design notes:

  • APPLY the analysis review comments to your analysis and work with them in mind!!!
  • Where is your test that tests the scripting logic??? That is the most important test... The demo is good idea though.
  • You know the script must be saveable with the book, so it's resource descriptor and js file must me storable the right way (see text resources persistence) in the R3.
  • Rename EventIds to ScriptEventIds if they are a separate file.

3.5p (50m)

comment:9 Changed 10 years ago by deni

  • Owner changed from mitex to deni
  • Status changed from s2c_design_ok to s3a_implementation_started

comment:10 Changed 10 years ago by mitex

  • Status changed from s3a_implementation_started to s3b_implementation_finished

comment:11 Changed 10 years ago by meddle

  • Status changed from s3b_implementation_finished to s3c_implementation_ok
  • Imp._score changed from 0 to 3.5
  • Imp._reviewers set to meddle, pap

Hmmm...

  • Some notes:
    • "if"s even one row must be with curly brackets
    • Bad spacing in some places.
    • The property book should be renamed to modelDocument and the AutoProperty with that name should be deleted.
    • The Persister of the script should persist it's scriptText using PropRef...
  • And there is a serious improvement in your coding and coding style.

3.5p (3h)

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