Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
[dsdp-tm-dev] RE: Your latest changes on SystemView (refresh parents of deleted objects)

Hi Kushal,

Now I see... thanks for the explanation.

What I still don't understand is this: When
the service returns "false" instead of throwing
an exception, would this still work as intended?

IFileService API is unclear whether delete()
and deleteBatch() may return false or throw
an exception.

What would you think about adding some of your
explanations as comments to the source code?

Thanks,
--
Martin Oberhuber
Wind River Systems, Inc.
Target Management Project Lead, DSDP PMC Member
http://www.eclipse.org/dsdp/tm 

> -----Original Message-----
> From: Kushal Munir [mailto:kmunir@xxxxxxxxxx] 
> Sent: Thursday, November 09, 2006 2:07 PM
> To: Oberhuber, Martin
> Cc: Target Management developer discussions
> Subject: Re: Your latest changes on SystemView (refresh 
> parents of deleted objects)
> 
> Hi Martin,
> 
> The reason for only iterating over the last "set" is that the 
> exception
> would occur for the last set when calling the doDeleteBatch() 
> method of the
> adapter, and we know that the sets before have worked (i.e. 
> not caused an
> exception). If a set is deleted successfully, the variable 
> anyOk is set to
> true and the contents of the set are added to the 
> deletedVector collection.
> Note that after the catch block there is this:
> 
> if (anyOk) {
>       if (selectionIsRemoteObject)
>             
> sr.fireRemoteResourceChangeEvent(ISystemRemoteChangeEvents.
> SYSTEM_REMOTE_RESOURCE_DELETED, deletedVector, null, null, 
> null, this);
> 
> So delete events are fired for the elements of all sets that 
> were deleted
> successfully. Delete events are more efficient than doing a 
> full refresh of
> the parents.
> 
> Let me know if you still think it's a problem.
> 
> Cheers,
> 
> Kushal Munir
> Websphere Development Studio Client for iSeries
> IBM Toronto Lab, 8200 Warden Ave., Markham, ON
> Phone: (905) 413-3118        Tie-Line: 969-3118
> Email: kmunir@xxxxxxxxxx
> 
> 
> 
>                                                               
>              
>              "Oberhuber,                                      
>              
>              Martin"                                          
>              
>              <Martin.Oberhuber                                
>           To 
>              @windriver.com>           Kushal 
> Munir/Toronto/IBM@IBMCA      
>                                                               
>           cc 
>              11/09/2006 05:27          "Target Management 
> developer        
>              AM                        discussions"           
>              
>                                        
> <dsdp-tm-dev@xxxxxxxxxxx>           
>                                                               
>      Subject 
>                                        Your latest changes on 
> SystemView   
>                                        (refresh parents of 
> deleted         
>                                        objects)               
>              
>                                                               
>              
>                                                               
>              
>                                                               
>              
>                                                               
>              
>                                                               
>              
>                                                               
>              
> 
> 
> 
> 
> Hi Kushal,
> 
> before I release the Mapfile for todays I-build, I reviewed
> all changes that happened yesterday, and I have a few questions
> regarding your changes on SystemView.doDelete(IProgressMonitor):
> 
> * One change is that while deleting resource sets, you now
>   remember the resource set being deleted:
> 
>                          SystemRemoteElementResourceSet set = null;
>                          try { /*...*/
>                                      for (int s = 0; s < 
> _setList.size() &&
> ok; s++)
> {
>                                                  set =
> (SystemRemoteElementResourceSet)
> _setList.get(s);
>             } /*...*/
> 
>   After these deletions, you iterate over the contents of "set"
>   in order to perform refreshes on the parents.
> 
> The problem I see with this is, that "set" is overwritten on each
> iteration again. So you end up refreshing the parents of the last
> member of _setList only.
> 
> This appears not to be the intention of the fix. My guess was that
> you'd rather want to refresh the parents of the union of all
> sets in the _setList. So I'd rather expect code like this:
> 
>                          SystemRemoteElementResourceSet allSets = new
> SystemRemoteElementResourceSet();
>                          try { /*...*/
>                                      for (int s = 0; s < 
> _setList.size() &&
> ok; s++)
> {
> 
> SystemRemoteElementResourceSet set =
> (SystemRemoteElementResourceSet) _setList.get(s);
>                                                  allSets.addAll(set);
>             } /*...*/
> 
> Was this what you want?
> 
> Thanks,
> --
> Martin Oberhuber
> Wind River Systems, Inc.
> Target Management Project Lead, DSDP PMC Member
> http://www.eclipse.org/dsdp/tm
> 
> >
> > * In org.eclipse.rse.services.files.ftp/Manifest.mf,
> >   you added a dependency to Eclipse UI code (ui.console, jface).
> >   I consider this very problematic since we want to keep the
> >   "services" layer UI-less. This is important in order to make
> >   sure that headless (UI-less) clients can use the services.
> >   Please try to get rid of this UI dependency.
> >
> >   The right solution for this might be that the consoleOutputStream
> >   is created in subsystems.files.ftp, and stored into the FTPservice
> >   when it is connected (e.g. FTPService.setLoggingStream())
> >
> >   Eventually, it might be a good idea to make logging part of API
> >   so we define IService.setLoggingStream() -- but I guess we'll
> >   need to think about this further.
> >
> > * FTPHostFile and FTPService used to compile without warnings,
> >   but now I see warnings in them - mostly missing $NON-NLS-1$ tags.
> >
> >   I know that the compiler warnings page at
> >   http://www.eclipse.org/dsdp/tm/development/compiler_warnings.php
> >   currently recommends "Ignore" for the $NON-NLS-1$ tags in most
> >   of RSE since it would overflood the log.
> >
> >   But for FTP, which used to be warning free, I think its a pity
> >   to lose that. Can you please adjust your warning settings to
> >   enable Warn on NON-NLS-1 and fix these?
> >
> > Thanks,
> > --
> > Martin Oberhuber
> > Wind River Systems, Inc.
> > Target Management Project Lead, DSDP PMC Member
> > http://www.eclipse.org/dsdp/tm
> 
> 
> 


Back to the top