[ 
http://issues.apache.org/jira/browse/FOR-571?page=comments#action_12315415 ] 

Ross Gardler commented on FOR-571:
----------------------------------

I've applied the patch with edits. 

The edits are javadoc comments, please ensure all code includes adequate 
javadoc comments (see 
http://marc.theaimsgroup.com/?l=forrest-dev&m=112080945118964&w=2 ). I've added 
the javadocs now, but it is much easier and faster for the original author to 
write them at the time they write the code or to modify them when they modify 
the code.. 

The dialog you have created in this patch should really be a separate class to 
facilitate reuse. You will find the refactoring tools of eclipse make this very 
easy to do. Further, I suggest that this should really be  wizard rather than a 
dialog, we are likely to want to expand this functionality quite considerably 
and I suspect that a dialog will not be sufficient. It's also much easier to 
create wizards than dialogs too :-)

Lastly (and least important). It pays to ensure your code is kept clean as this 
makes for more readable code in the long run. Eclipse provides many tools for 
doing this, one of them is source -> organize imports context menu option. This 
will remove unused imports and expand wildcard imports.

----

All the above comments (with the exception of Javadocs) are intended to improve 
the code, not as criticism. This is Open Source, others can help and improve 
your initial code. The code you have submitted works, that is more than we had 
before so is great progress.

Thanks!

> Added dialog to fill in the attributes of a new element the site.xml file
> -------------------------------------------------------------------------
>
>          Key: FOR-571
>          URL: http://issues.apache.org/jira/browse/FOR-571
>      Project: Forrest
>         Type: New Feature
>   Components: Tool: Eclipse config
>     Versions: 0.8-dev
>     Reporter: Anil Ramnanan
>  Attachments: SiteXML090705.Diff
>
> This is for a patch with a  dialog that allows you to fill in the information 
> for a new element such as the element name, href and description.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira

Reply via email to