Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 355220

Summary: [modeling] separate internal namespace from API
Product: z_Archived Reporter: Steffen Pingel <steffen.pingel>
Component: MylynAssignee: Miles Parker <milesparker>
Status: RESOLVED FIXED QA Contact:
Severity: enhancement    
Priority: P3    
Version: unspecified   
Target Milestone: 0.9   
Hardware: All   
OS: All   
Whiteboard:
Bug Depends on:    
Bug Blocks: 355025    

Description Steffen Pingel CLA 2011-08-19 09:30:08 EDT
All classes that are not considered API should have a internal segment in the package name, e.g. org.eclipse.mylyn.internal.modeling.emf (instead of org.eclipse.mylyn.modeling.emf). Internal package must be exported as x-internal in the manifest. 

Classes intended to be extended by integrators should be moved to an spi package.
Comment 1 Miles Parker CLA 2011-08-19 15:57:00 EDT
(In reply to comment #0)
> All classes that are not considered API should have a internal segment in the
> package name, e.g. org.eclipse.mylyn.internal.modeling.emf (instead of
> org.eclipse.mylyn.modeling.emf). Internal package must be exported as x-internal
> in the manifest.

Note that this isn't obvious that many classes are intended for extension or direct use.

> Classes intended to be extended by integrators should be moved to an spi
> package.

OK, I found this interesting wiki: http://wiki.eclipse.org/SPI_description

"The defining characteristic of SPI is that it's special API for a very particular, advanced client, and not intended for general client use. It may impose specific restrictions that go beyond the general API usage contract of the platform."

I think that there isn't anything appropriate for SPI, but this is sort of a gray area. It' very common in EMF/GMF for people to extend these kinds of things, providing meta-data and so on. I'd like to actually make these even more straightforward. But the idea is that anyone who is creating an M1 or M2 model (http://en.wikipedia.org/wiki/Meta-Object_Facility) would want to extend the functionality. In fact, we'd like to provide an extension for that, but it doesn't seem possible right now. One of the things I'm wanting to do is clean up the API even further and we might imagine using code generation tools to create all of the artifacts. See o.e.m.modeling.papyrus as an exemplar -- aside from the plumbing in internal which is all boiler plate, the user simply has to provide their domain mappings e.g. Uml2DomainBridge.

So in general, I think these have a far wider audience of consumers than say someone who is creating a Mylyn implementation for some large target tooling.
Comment 2 Miles Parker CLA 2011-08-19 15:58:37 EDT
https://github.com/MilesParker/mylyn.incubator/commit/1c82a17b7ff5bc1f0257332185ef3845c1ac1894. I'm going to resolve but please reopen if you have differing view re: SPI issue. :)
Comment 3 Steffen Pingel CLA 2011-08-19 17:59:40 EDT
(In reply to comment #1)
> (In reply to comment #0)
> > All classes that are not considered API should have a internal segment in the
> > package name, e.g. org.eclipse.mylyn.internal.modeling.emf (instead of
> > org.eclipse.mylyn.modeling.emf). Internal package must be exported as
> x-internal
> > in the manifest.
> 
> Note that this isn't obvious that many classes are intended for extension or
> direct use.

Can you clarify what you mean by that? For API classes this should be specified in the JavaDoc comment for the class.
Comment 4 Steffen Pingel CLA 2011-08-19 18:03:27 EDT
Thanks for making these changes. It looks like the naming of the internal package of the papyrus and gmf bundles do not follow the standard naming guideline.

What is the reason for making all the bridges API? Wouldn't it suffice if the interfaces IModelStructureProvider and IModelUiProvider were API?
Comment 5 Miles Parker CLA 2011-08-19 18:18:06 EDT
(In reply to comment #4)
> Thanks for making these changes. It looks like the naming of the internal
> package of the papyrus and gmf bundles do not follow the standard naming
> guideline.

Do you mean where internal is in the namespace segment? That seems to be again all over the map in Eclipse land. Modeling projects tend to put them toward the end. So you want e.g. "o.e.mylyn.internal.modeling.papyrus"?

> 
> What is the reason for making all the bridges API? Wouldn't it suffice if the
> interfaces IModelStructureProvider and IModelUiProvider were API?

No. For example, someone may want to extend the EmfStructureBridge so that it can be generalized in different ways, just as I've done with the GmfStructureBridge and in turn one might want to do the same with that class. That's no different than say for AbstractContextStructureBridge.
Comment 6 Miles Parker CLA 2011-08-19 18:20:10 EDT
Note that I have placed the "end-point" bridges, e.g. Uml2StructureBridge as internal.
Comment 7 Miles Parker CLA 2011-08-19 18:28:32 EDT
(In reply to comment #5)
> (In reply to comment #4)
> > Thanks for making these changes. It looks like the naming of the internal
> > package of the papyrus and gmf bundles do not follow the standard naming
> > guideline.

btw, I'm not sure if the Mylyn guidelines are captured anywhere? I must say that my own project only has three contributors so we've been a little loose on these things, but I've had exactly the same conversations with them -- I do think consistency is important. I've only looked closely at http://wiki.eclipse.org/Mylyn_Contributor_Reference. Or perhaps these are Eclipse-wide guidelines and folks are just ignoring them. :) I could/should just scan the other Mylyn code better but it would be easier to have a reference.
Comment 8 Steffen Pingel CLA 2011-08-19 20:31:22 EDT
(In reply to comment #5)
> (In reply to comment #4)
> > Thanks for making these changes. It looks like the naming of the internal
> > package of the papyrus and gmf bundles do not follow the standard naming
> > guideline.
> 
> Do you mean where internal is in the namespace segment? That seems to be again
> all over the map in Eclipse land. Modeling projects tend to put them toward the
> end. So you want e.g. "o.e.mylyn.internal.modeling.papyrus"?

Yes, that's the convention that's commonly used in Mylyn.

> > What is the reason for making all the bridges API? Wouldn't it suffice if the
> > interfaces IModelStructureProvider and IModelUiProvider were API?
> 
> No. For example, someone may want to extend the EmfStructureBridge so that it
> can be generalized in different ways, just as I've done with the
> GmfStructureBridge and in turn one might want to do the same with that class.
> That's no different than say for AbstractContextStructureBridge.

Good. That makes sense. The bridge classes would likely go into an SPI package then and we probably want to consider converting the interfaces to abstract classes or at least to provide default implementations that are to be extended by integrators.
Comment 9 Steffen Pingel CLA 2011-08-19 20:43:15 EDT
(In reply to comment #7)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Thanks for making these changes. It looks like the naming of the internal
> > > package of the papyrus and gmf bundles do not follow the standard naming
> > > guideline.
> 
> btw, I'm not sure if the Mylyn guidelines are captured anywhere? I must say that
> my own project only has three contributors so we've been a little loose on these
> things, but I've had exactly the same conversations with them -- I do think
> consistency is important. I've only looked closely at
> http://wiki.eclipse.org/Mylyn_Contributor_Reference. Or perhaps these are
> Eclipse-wide guidelines and folks are just ignoring them. :) I could/should just
> scan the other Mylyn code better but it would be easier to have a reference.

We enforce the project guidelines fairly strictly. Some of the policies are documented in the contributors reference or enforced by automatic settings. Otherwise you can deduct them from existing bundles. We have established the guidelines over time and I would assume that most of them are specific to Mylyn.

It's one of the main reason for the high contribution threshold to gain committer rights. The review and feedback process that's triggered by every single patch creates a shared understanding of the guidelines and policies. 

Please don't misunderstand my nit picking as criticism. It's a natural part of getting any contribution accepted into Mylyn and it's hopefully fairly trivial to make the requested changes. Having a consistent code base helps tremendously with shared code ownership and maintenance.
Comment 10 Miles Parker CLA 2011-08-19 22:17:11 EDT
(In reply to comment #9)
> Please don't misunderstand my nit picking as criticism.

Not at all and I appreciate that. It is interesting to notice how one ends up feeling defensive even when intellectually we know there is no good reason to be. :)
Comment 11 Miles Parker CLA 2011-08-31 13:05:01 EDT
OK I think I fixed this. Can we hold off on or open a seperate bug for spi issue?
Comment 12 Steffen Pingel CLA 2011-08-31 14:50:06 EDT
Sure, we can close this and open new bugs for any remaining discussion items.