[dsdp-tm-dev] Your latest changes on SystemView (refresh parents of dele

["Oberhuber, Martin" <Martin.Oberhuber@xxxxxxxxxxxxx>]

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