Bug 274688 - [api][dstore] DStoreConnectorService.internalConnect() needs to be cleaned up
Summary: [api][dstore] DStoreConnectorService.internalConnect() needs to be cleaned up
Status: RESOLVED FIXED
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 3.0.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.1 M7   Edit
Assignee: David McKnight CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2009-05-01 15:24 EDT by David McKnight CLA
Modified: 2009-05-04 08:44 EDT (History)
0 users

See Also:
mober.at+eclipse: pmc_approved+


Attachments
patch with splitting up of internalConnect() (64.92 KB, patch)
2009-05-01 15:29 EDT, David McKnight CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David McKnight CLA 2009-05-01 15:24:33 EDT
DStoreConnectorService.internalConnect() is pretty long and ugly to read.  This ought to be cleaned up such that each server launch type (daemon, rexec, manual, etc.) has it's own overrideable method.  Because these methods should be overrideable, this would introduce new protected methods - hence the API tag.
Comment 1 David McKnight CLA 2009-05-01 15:29:13 EDT
Created attachment 134105 [details]
patch with splitting up of internalConnect()

Martin, because of the new protected methods, this constitutes an API change.  Is there any way, that we can get a change like this into 3.1 or will such changes have to wait?
Comment 2 Martin Oberhuber CLA 2009-05-02 17:08:07 EDT
Hm... I don't have much of a problem with adding API at this point as long as no API is broken. Unfortunately the patch is hard to read so I can't really see the changes. 

Is there any real business case behind making those new split up methods "protected"? Who would ever override dstoreConnectorService and override any of these methods? Could they also just be private?

Usually, Eclipse Projects don't add API if there is no client for that API known.
Comment 3 David McKnight CLA 2009-05-03 11:07:03 EDT
(In reply to comment #2)
> Hm... I don't have much of a problem with adding API at this point as long as
> no API is broken. Unfortunately the patch is hard to read so I can't really see
> the changes. 
> 
> Is there any real business case behind making those new split up methods
> "protected"? Who would ever override dstoreConnectorService and override any of
> these methods? Could they also just be private?
> 
> Usually, Eclipse Projects don't add API if there is no client for that API
> known.
> 

There are a couple IBM products that extend DStoreConnectorService to provide certain customizations.  By splitting this stuff up and making them protected, it's easier for these products to make their customizations.
Comment 4 Martin Oberhuber CLA 2009-05-03 14:05:46 EDT
ok, go for it
Comment 5 David McKnight CLA 2009-05-04 08:44:30 EDT
I've committed the change to cvs.