[dsdp-tm-dev] RE: RC1 Code Review - Refresh

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

But then checking for IRemoteObjectIdentifier is not right,

since SystemViewElementAdapter also implements that.

 

The check needs to be more elaborate.

 

Or we change API and invent yet another markup interface

for remote stuff.

 

But I think it's OK to do this for RC2 only.

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

 

---

From: David McKnight [mailto:dmcknigh@xxxxxxxxxx]
Sent: Friday, May 25, 2007 8:05 PM
To: Oberhuber, Martin
Cc: Target Management developer discussions
Subject: Re: RC1 Code Review - Refresh

Hi Martin,

I can change the delete action to log the exception.

For the remote adapter stuff, I think using the remote object identifier should be sufficient to deal with these cases.  I just don't want filters, subsystems and connections to have to go through that logic.  I'll change that code now.

Thanks,

____________________________________
David McKnight    
Phone:   905-413-3902 , T/L:  969-3902
Internet: dmcknigh@xxxxxxxxxx
Mail:       D1/YFY/8200/TOR
____________________________________

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

25/05/2007 01:58 PM

To	David McKnight/Toronto/IBM@IBMCA
cc	"Target Management developer discussions" <dsdp-tm-dev@xxxxxxxxxxx>
Subject	RC1 Code Review - Refresh

Hi Dave,

I reviewed yesterday's changes and I have some questions
and requests:

* SystemCommonDeleteAction:
 you added e.printStackTrace()
 We should avoid this wherever possible. Errors should
 be logged instead of printing to stdout.

* SystemView.EVENT_REFRESH_REMOTE:
 [187732] can only do the reget of "remote" items
 You added if(getRemoteAdapter(src)!=null)
 This is a very dangerous change IMHO and may break our
 commercial product... is there really a need to implement
 ISystemRemoteElementAdapter? Wouldn't it be sufficient
 to implement IRemoteObjectIdentifier or ISystemViewElementAdapter?

 We do not implement ISystemRemoteElementAdapter in our commercial
 product, and we filed some bugs because of this earlier.
 That's why a lot of code in SystemView now uses the
 SYstemViewElementAdapter rather than ISystemRemoteElementAdapter.

 But perhaps I'm misunderstanding things?
 My concept of something being "Remote" is if
   1.) It implements ISystemRemoteObjectIdentifier
   2.) It is associated with a subsystem
       adapter.getSubSystem(Object) != null

 What do you think?

Can you please answer and/or address these.

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