> On Aug. 10, 2012, 8:41 a.m., Virag Kothari wrote:
> > trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/OozieWorkflowGenerator.java,
> >  line 109
> > <https://reviews.apache.org/r/6435/diff/2/?file=136383#file136383line109>
> >
> >     why not list and reuse the same in property table?

yes good point. agree. we can remove widgetlist in each property table.
original intention of using set was to avoid duplicated insertion by mistake.


> On Aug. 10, 2012, 8:41 a.m., Virag Kothari wrote:
> > trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/OozieWorkflowGenerator.java,
> >  line 278
> > <https://reviews.apache.org/r/6435/diff/2/?file=136383#file136383line278>
> >
> >     instead of looping through all the widgets to obtain the name of widget 
> > in selection, can we have a Map of <TypeOfNode, count> and then use this 
> > count to derive the name of the widget.

valid point, I'm thinking of pros and cons. 
once we keep <Type of Node, count> data structure, we have to keep it 
consistent every time node widget added or removed (meaning other parts of code 
need to be changed as well),  so far only this method requires count of each 
type, not other parts. (also only a few node widgets created in workflow at 
most, impact of performance is not that bad) probably we can add it when other 
part of code start using count of each type?


> On Aug. 10, 2012, 8:41 a.m., Virag Kothari wrote:
> > trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/PropertyTable.java,
> >  line 55
> > <https://reviews.apache.org/r/6435/diff/2/?file=136385#file136385line55>
> >
> >     I see that this widgetList is only used to obtain the kill node. The 
> > kill node name can be obtained from the  generator. Is this list required?
> >     The widget drop down list is already present to get the drop downs for 
> > properties.
> >     
> >

agree with your point of removing widgetlist.


> On Aug. 10, 2012, 8:41 a.m., Virag Kothari wrote:
> > trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/PropertyTable.java,
> >  line 76
> > <https://reviews.apache.org/r/6435/diff/2/?file=136385#file136385line76>
> >
> >     can we reuse the widgetList of generator itself instead of copying it 
> > over to a new list?..In that case we dont need to call this update* methods 
> > on each event.

same with above, agree with removing widgetlist


> On Aug. 10, 2012, 8:41 a.m., Virag Kothari wrote:
> > trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/action/EmailActionWidget.java,
> >  line 42
> > <https://reviews.apache.org/r/6435/diff/2/?file=136404#file136404line42>
> >
> >     this is being done in initPropertyTable()
> >     why again here?

initPropertyTable is called only when this node widget created. 
onMouseDown is called whenever this node widget clicked in a workflow design 
panel.
generator keeps property table of previously-clicked node widget, thus we have 
to update it every time


- Ryota


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6435/#review10110
-----------------------------------------------------------


On Aug. 11, 2012, 12:17 a.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6435/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2012, 12:17 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-944
> Please note this is a big patch including 71 files total.
> 38 java files, 9 config files (maven related files/GWT related XML/readme, 
> etc), 24 images files (since this is UI tool. should be excluded for review)
> 
> 
> This addresses bug OOZIE-944.
>     https://issues.apache.org/jira/browse/OOZIE-944
> 
> 
> Diffs
> -----
> 
>   trunk/workflowgenerator/README.txt PRE-CREATION 
>   trunk/workflowgenerator/pom.xml PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/OozieDiagramController.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/OozieWorkflowGenerator.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/Property.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/PropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/action/EmailPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/action/FSPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/action/JavaPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/action/MapReducePropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/action/PigPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/action/PipesPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/action/SSHPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/action/ShellPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/action/StreamingPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/action/SubWFPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/control/DecisionPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/control/EndPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/control/ForkPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/control/JoinPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/control/KillPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/control/StartPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/property/control/WrkflowPropertyTable.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/NodeWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/action/EmailActionWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/action/FSActionWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/action/JavaActionWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/action/MapReduceActionWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/action/PigActionWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/action/PipesActionWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/action/SSHActionWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/action/ShellActionWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/action/StreamingActionWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/action/SubWFActionWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/control/DecisionNodeWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/control/EndNodeWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/control/ForkNodeWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/control/JoinNodeWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/control/KillNodeWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/client/widget/control/StartNodeWidget.java
>  PRE-CREATION 
>   
> trunk/workflowgenerator/src/main/java/org/apache/oozie/tools/workflowgenerator/workflowgenerator.gwt.xml
>  PRE-CREATION 
>   trunk/workflowgenerator/src/main/resources/img/action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/add-btn.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/decision.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/del-btn.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/distcp-action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/email-action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/end.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/fork-shape.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/fork.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/fs-action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/hive-action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/java-action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/join-shape.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/join.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/kill.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/mr-action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/pig-action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/pipes-action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/shell-action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/ssh-action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/start-shape.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/start.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/streaming-action.png UNKNOWN 
>   trunk/workflowgenerator/src/main/resources/img/subwf-action.png UNKNOWN 
>   
> trunk/workflowgenerator/src/main/resources/org/apache/oozie/tools/workflowgenerator/workflowgenerator.gwt.xml
>  PRE-CREATION 
>   trunk/workflowgenerator/src/main/webapp/WEB-INF/web.xml PRE-CREATION 
>   trunk/workflowgenerator/src/main/webapp/workflowgenerator.css PRE-CREATION 
>   trunk/workflowgenerator/src/main/webapp/workflowgenerator.html PRE-CREATION 
>   trunk/workflowgenerator/workflowgeneratorTest-dev.launch PRE-CREATION 
>   trunk/workflowgenerator/workflowgeneratorTest-prod.launch PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/6435/diff/
> 
> 
> Testing
> -------
> 
> locally run and check UI components on browser
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>

Reply via email to